[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1240958794.938.1045.camel@calx>
Date: Tue, 28 Apr 2009 17:46:34 -0500
From: Matt Mackall <mpm@...enic.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Wu Fengguang <fengguang.wu@...el.com>,
linux-kernel@...r.kernel.org, kosaki.motohiro@...fujitsu.com,
andi@...stfloor.org, adobriyan@...il.com, linux-mm@...ck.org
Subject: Re: [PATCH 5/5] proc: export more page flags in /proc/kpageflags
On Tue, 2009-04-28 at 14:32 -0700, Andrew Morton wrote:
> > +#define kpf_copy_bit(uflags, kflags, visible, ubit, kbit) \
> > + do { \
> > + if (visible || genuine_linus()) \
> > + uflags |= ((kflags >> kbit) & 1) << ubit; \
> > + } while (0);
>
> Did this have to be implemented as a macro?
I'm mostly to blame for that. I seem to recall the optimizer doing a
better job on this as a macro.
> It's bad, because it might or might not reference its argument, so if
> someone passes it an expression-with-side-effects, the end result is
> unpredictable. A C function is almost always preferable if possible.
I don't think there's any use case for it outside of its one user?
> > +/* a helper function _not_ intended for more general uses */
> > +static inline int page_cap_writeback_dirty(struct page *page)
> > +{
> > + struct address_space *mapping;
> > +
> > + if (!PageSlab(page))
> > + mapping = page_mapping(page);
> > + else
> > + mapping = NULL;
> > +
> > + return mapping && mapping_cap_writeback_dirty(mapping);
> > +}
>
> If the page isn't locked then page->mapping can be concurrently removed
> and freed. This actually happened to me in real-life testing several
> years ago.
We certainly don't want to be taking locks per page to build the flags
data here. As we don't have any pretense of being atomic, it's ok if we
can find a way to do the test that's inaccurate when a race occurs, so
long as it doesn't dereference null.
But if there's not an obvious way to do that, we should probably just
drop this flag bit for this iteration.
--
http://selenic.com : development and support for Mercurial and Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists