<div dir="ltr">Returning ENOSPC when you have free space you can't yet prove is safer than not returning it and risking a data loss when you get hit by a write/commit storm. :)</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 30, 2015 at 9:44 PM, OGAWA Hirofumi <span dir="ltr"><<a href="mailto:hirofumi@mail.parknet.co.jp" target="_blank">hirofumi@mail.parknet.co.jp</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Jan Kara <<a href="mailto:jack@suse.cz">jack@suse.cz</a>> writes:<br>
<br>
>> > Yes, if userspace truncates the file, the situation we end up with is<br>
>> > basically the same. However for truncate to happen some malicious process<br>
>> > has to come and truncate the file - a failure scenario that is acceptable<br>
>> > for most use cases since it doesn't happen unless someone is actively<br>
>> > trying to screw you. With page forking it is enough for flusher thread<br>
>> > to start writeback for that page to trigger the problem - event that is<br>
>> > basically bound to happen without any other userspace application<br>
>> > interfering.<br>
>><br>
>> Acceptable conclusion is where came from? That pseudocode logic doesn't<br>
>> say about usage at all. And even if assume it is acceptable, as far as I<br>
>> can see, for example /proc/sys/vm/drop_caches is enough to trigger, or a<br>
>> page on non-exists block (sparse file. i.e. missing disk space check in<br>
>> your logic). And if really no any lock/check, there would be another<br>
>> races.<br>
><br>
> So drop_caches won't cause any issues because it avoids mmaped pages.<br>
> Also page reclaim or page migration don't cause any issues because<br>
> they avoid pages with increased refcount (and increased refcount would stop<br>
> drop_caches from reclaiming the page as well if it was not for the mmaped<br>
> check before). Generally, elevated page refcount currently guarantees page<br>
> isn't migrated, reclaimed, or otherwise detached from the mapping (except<br>
> for truncate where the combination of mapping-index becomes invalid) and<br>
> your page forking would change that assumption - which IMHO has a big<br>
> potential for some breakage somewhere.<br>
<br>
</span>Lifetime and visibility from user are different topic.  The issue here<br>
is visibility. Of course, those has relation more or less though,<br>
refcount doesn't stop to drop page from radix-tree at all.<br>
<br>
Well, anyway, your claim seems to be assuming the userspace app<br>
workarounds the issues. And it sounds like still not workarounds the<br>
ENOSPC issue (validate at page fault/GUP) even if assuming userspace<br>
behave as perfect. Calling it as kernel assumption is strange.<br>
<br>
If you claim, there is strange logic widely used already, and of course,<br>
we can't simply break it because of compatibility. I would be able to<br>
agree. But your claim sounds like that logic is sane and well designed<br>
behavior. So I disagree.<br>
<span class=""><br>
> And frankly I fail to see why you and Daniel care so much about this<br>
> corner case because from performance POV it's IMHO a non-issue and you<br>
> bother with page forking because of performance, don't you?<br>
<br>
</span>Trying to penalize the corner case path, instead of normal path, should<br>
try at first. Penalizing normal path to allow corner case path is insane<br>
basically.<br>
<br>
Make normal path faster and more reliable is what we are trying.<br>
<span class=""><br>
> So you can have a look for example at<br>
> drivers/media/v4l2-core/videobuf2-dma-contig.c which implements setting up<br>
> of a video device buffer at virtual address specified by user. Now I don't<br>
> know whether there really is any userspace video program that sets up the<br>
> video buffer in mmaped file. I would agree with you that it would be a<br>
> strange thing to do but I've seen enough strange userspace code that I<br>
> would not be too surprised.<br>
><br>
> Another example of similar kind is at<br>
> drivers/infiniband/core/umem.c where we again set up buffer for infiniband<br>
> cards at users specified virtual address. And there are more drivers in<br>
> kernel like that.<br>
<br>
</span>Unfortunately, I'm not looking those yet though. I guess those would be<br>
helpful to see the details.<br>
<span class="im HOEnZb"><br>
Thanks.<br>
--<br>
OGAWA Hirofumi <<a href="mailto:hirofumi@mail.parknet.co.jp">hirofumi@mail.parknet.co.jp</a>><br>
<br>
</span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
Tux3 mailing list<br>
<a href="mailto:Tux3@phunq.net">Tux3@phunq.net</a><br>
<a href="http://phunq.net/mailman/listinfo/tux3" rel="noreferrer" target="_blank">http://phunq.net/mailman/listinfo/tux3</a><br>
</div></div></blockquote></div><br></div>