lkml.org 
[lkml]   [2009]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/5] proc: export more page flags in /proc/kpageflags
From
Date
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




\
 
 \ /
  Last update: 2009-04-29 00:51    [from the cache]
©2003-2011 Jasper Spaans