Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Sat, Dec 12 2015, Al Viro wrote: > On Fri, Dec 11, 2015 at 11:16:24PM +, Al Viro wrote: > >> #define set_delayed_type(call, f, arg) \ >> sizeof(f(arg),0), \ >> __set_delayed_type(call, __closure_##f, (void *)arg) >> >> That could be reused for timers with typechecking - we have

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Sat, Dec 12 2015, Al Viro wrote: > On Fri, Dec 11, 2015 at 11:16:24PM +, Al Viro wrote: > >> #define set_delayed_type(call, f, arg) \ >> sizeof(f(arg),0), \ >> __set_delayed_type(call, __closure_##f, (void *)arg) >> >> That could be reused for timers

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 01:54:25AM +, Al Viro wrote: > BTW, why are we passing unsigned long to free_page()? We have > a bit under 700 callers; excluding the ones that have an explicit cast > to unsigned long right in the argument of call leaves ~150, and the rest > tend to contain a

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 01:54:25AM +, Al Viro wrote: > BTW, why are we passing unsigned long to free_page()? We have > a bit under 700 callers; excluding the ones that have an explicit cast > to unsigned long right in the argument of call leaves ~150, and the rest > tend to contain a

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 11:16:24PM +, Al Viro wrote: > #define set_delayed_type(call, f, arg) \ > sizeof(f(arg),0), \ > __set_delayed_type(call, __closure_##f, (void *)arg) > > That could be reused for timers with typechecking - we have a lot of timer > callbacks that start with

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 08:49:24AM +0100, Rasmus Villemoes wrote: > union delayed_call_fn { > void (*fn)(void *); > void (*kfree_like)(const void *); > } __attribute__((__transparent_union__)); > > void > set_delayed_call(struct delayed_call *call, union delayed_call_fn u, void >

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 08:49:24AM +0100, Rasmus Villemoes wrote: > union delayed_call_fn { > void (*fn)(void *); > void (*kfree_like)(const void *); > } __attribute__((__transparent_union__)); > > void > set_delayed_call(struct delayed_call *call, union delayed_call_fn u, void >

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11, 2015 at 11:16:24PM +, Al Viro wrote: > #define set_delayed_type(call, f, arg) \ > sizeof(f(arg),0), \ > __set_delayed_type(call, __closure_##f, (void *)arg) > > That could be reused for timers with typechecking - we have a lot of timer > callbacks that start with

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11 2015, Al Viro wrote: > I would really love to be able to say > set_delayed_call(done, kfree, p); > but as it is I had to keep a wrapper - void kfree_link(void *). The problem > is, you can't assign void f(const void *) to void (*p)(void *) - mismatch of > qualifiers in the

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Thu, Dec 10, 2015 at 02:40:50AM +, Al Viro wrote: > ... except that it would have to be something like work_struct, since > callback_head gets embedded into the argument and here we don't want to > do that. _And_ we've no use for ->entry in work_struct, which makes it > a bad fit as well.

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Thu, Dec 10, 2015 at 02:40:50AM +, Al Viro wrote: > ... except that it would have to be something like work_struct, since > callback_head gets embedded into the argument and here we don't want to > do that. _And_ we've no use for ->entry in work_struct, which makes it > a bad fit as well.

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Fri, Dec 11 2015, Al Viro wrote: > I would really love to be able to say > set_delayed_call(done, kfree, p); > but as it is I had to keep a wrapper - void kfree_link(void *). The problem > is, you can't assign void f(const void *) to void (*p)(void *) -

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Thu, Dec 10, 2015 at 12:10:12AM +, Al Viro wrote: > PS: I toyed with the idea of replacing cookie a struct callback_head, to make ... except that it would have to be something like work_struct, since callback_head gets embedded into the argument and here we don't want to do that. _And_

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Wed, Dec 09, 2015 at 06:23:09PM +, Al Viro wrote: > What's more, that dentry might very well have gone negative by that > point. Think what happens if, during the symlink traversal, we run > into the hard "restart from scratch in non-RCU mode". We'll need to > do ->put_link() on

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

Hi Al, thanks a lot for this. I particularly like that you renamed "follow_link" which didn't really describe what the function did any more. Apart from the changelog comment in 4/11 which starts "It was to needed for ..." I can't fault anything. Thanks, NeilBrown signature.asc

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Wed, Dec 09, 2015 at 09:24:32AM -0800, Linus Torvalds wrote: >That's because of the HIGHMEM crap that you removed. Without > highmem, the page data address specifies the page fine without any > cookie. > > - overlayfs, which uses it to create a temporary data storage. > >Why? Mainly

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Tue, Dec 8, 2015 at 9:32 PM, Al Viro wrote: > > Any help with review and testing would be welcome. FWIW, it seems to survive > the beating here. It looks ok to me, but I *hate* that "cookie" thing we have. And I don't actually see any reason for it. Every single user except for a couple

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Tue, Dec 8, 2015 at 9:32 PM, Al Viro wrote: > > Any help with review and testing would be welcome. FWIW, it seems to survive > the beating here. It looks ok to me, but I *hate* that "cookie" thing we have. And I don't actually see any reason for it. Every single

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Wed, Dec 09, 2015 at 09:24:32AM -0800, Linus Torvalds wrote: >That's because of the HIGHMEM crap that you removed. Without > highmem, the page data address specifies the page fine without any > cookie. > > - overlayfs, which uses it to create a temporary data storage. > >Why? Mainly

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

Hi Al, thanks a lot for this. I particularly like that you renamed "follow_link" which didn't really describe what the function did any more. Apart from the changelog comment in 4/11 which starts "It was to needed for ..." I can't fault anything. Thanks, NeilBrown signature.asc

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Wed, Dec 09, 2015 at 06:23:09PM +, Al Viro wrote: > What's more, that dentry might very well have gone negative by that > point. Think what happens if, during the symlink traversal, we run > into the hard "restart from scratch in non-RCU mode". We'll need to > do ->put_link() on

Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Thu, Dec 10, 2015 at 12:10:12AM +, Al Viro wrote: > PS: I toyed with the idea of replacing cookie a struct callback_head, to make ... except that it would have to be something like work_struct, since callback_head gets embedded into the argument and here we don't want to do that. _And_

[PATCHSET v2] ->follow_link() without dropping from RCU mode

On Tue, Nov 17, 2015 at 10:57:52PM +, Al Viro wrote: > Right now we stay in RCU mode for fast symlink traversal. > However, anything trickier drops out of RCU mode - back in 4.2 > the symlink-related pile had grown too large to add this on top > of everything else. Below is an attempt

[PATCHSET v2] ->follow_link() without dropping from RCU mode

On Tue, Nov 17, 2015 at 10:57:52PM +, Al Viro wrote: > Right now we stay in RCU mode for fast symlink traversal. > However, anything trickier drops out of RCU mode - back in 4.2 > the symlink-related pile had grown too large to add this on top > of everything else. Below is an attempt