Re: [PATCH] dax: Fix Xarray conversion of dax_unlock_mapping_entry()

From: Matthew Wilcox
Date: Fri Nov 30 2018 - 14:50:26 EST


On Fri, Nov 30, 2018 at 09:01:07AM -0800, Dan Williams wrote:
> On Fri, Nov 30, 2018 at 8:33 AM Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> >
> > On Fri, Nov 30, 2018 at 8:24 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > >
> > > On Fri, Nov 30, 2018 at 07:54:49AM -0800, Dan Williams wrote:
> > > > Looks good to me, although can we make that cookie an actual type? I
> > > > think it's mostly ok to pass around (void *) for 'entry' inside of
> > > > fs/dax.c, but once an entry leaves that file I'd like it to have an
> > > > explicit type to catch people that might accidentally pass a (struct
> > > > page *) to the unlock routine.
> > >
> > > That's a really good idea. Something like this?
> > >
> > > typedef struct {
> > > void *v;
> > > } dax_entry_t;
> >
> > Yes, please.

Oh. The caller needs to interpret it to see if the entry was successfully
locked, so it needs to be an integer type (or we need a comparison
function ... bleh).

> > > I could see us making good use of that within dax.c.
>
> I'm now thinking that this is a nice improvement for 4.21. For 4.20-rc
> lets do the localized fix.

I think both patches are equally risky. I admit this patch crosses a
file boundary, but the other patch changes dax_make_entry() which is
used by several functions which aren't part of this path, whereas this
patch only changes functions used in the path which is known to be buggy.

This patch has the advantage of getting us closer to where we want to be
sooner.