On Sat, Feb 4, 2017 at 4:08 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Feb 02, 2017 at 09:51:25AM +0000, Al Viro wrote:
>
>> * fuse_copy_fill(). I'm not at all sure that iov_iter_get_pages()
>> is a good idea there - fuse_copy_do() could bloody well just use
>> copy_{to,from}_iter().
>
> Miklos, could you explain why does lock_request() prohibit page faults until
> the matching unlock_request()? All it does is setting FR_LOCKED on
> our request and the only thing that even looks at that is fuse_abort_conn(),
> which doesn't (AFAICS) wait for anything.
>
> Where does the deadlock come from, and if it's not a deadlock - what is
> it? Or is that comment stale since "fuse: simplify request abort"?
Well, it's not historical; at least not yet. The deadlock is there
alright: mmap fuse file to addr; read byte from mapped page -> page
locked; this triggeres read request served in same process but
separate thread; write addr-headerlen to fuse dev; trying to lock same
page -> deadlock.
The deadlock can be broken by aborting or force unmounting: return
error for original read request; page unlocked; device write can get
page lock and return.
The reason we need to prohibit pagefault while copying is that when
request is aborted and the caller returns the memory in the request
may become invalid (e.g. data from stack).
Another solution would be to copy all data and keep a ref on the copy
by the request even after being aborted. This is the plan for the
future.
Thanks,
Miklos
