lkml.org 
[lkml]   [2022]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: linux-next: build warnings after merge of the akpm-current tree
On Wed, Feb 09, 2022 at 08:03:26AM -0800, Hugh Dickins wrote:
> On Wed, 9 Feb 2022, Stephen Rothwell wrote:
> > include/linux/mm_types.h:272: warning: Function parameter or member '__filler' not described in 'folio'
> > include/linux/mm_types.h:272: warning: Function parameter or member 'mlock_count' not described in 'folio'
>
> Thank you for including the patches and reporting this, Stephen.
> Is this a warning you can live with for a week or two?
>
> I've never tried generating htmldocs (I'm tempted just to replace a few
> "/**"s by "/*"s!), and I'm fairly sure Matthew will have strong feelings
> about how this new union (or not) will be better foliated - me messing
> around with doc output here is unlikely to be helpful at this moment.

I have a substantial question, and then some formatting / appearance
questions.

The first is, what does mlock_count represent for a multi-page folio
that is partially mlocked? If you allocate an order-9 page then mmap()
13 pages of it and mlockall(), does mlock_count increase by 1, 13 or 512?

Then we have a tradeoff between prettiness of the source code and
prettiness of the htmldocs. At one maximum, we can make it look like
this in the htmldocs:

struct folio {
unsigned long flags;
struct list_head lru;
unsigned int mlock_count;
struct address_space *mapping;
pgoff_t index;
void *private;
atomic_t _mapcount;
atomic_t _refcount;
#ifdef CONFIG_MEMCG;
unsigned long memcg_data;
#endif;
};

but at the cost of making the source code look like this:

struct folio {
/* private: don't document the anon union */
union {
struct {
/* public: */
unsigned long flags;
/* private: */
union {
/* public: */
struct list_head lru;
/* private: */
struct {
void *__filler;
/* public: */
unsigned int mlock_count;
/* private: */
};
};
/* public: */
struct address_space *mapping;

At the other extreme, the htmldocs can look more complicated:

struct folio {
unsigned long flags;
union {
struct list_head lru;
struct {
unsigned int mlock_count;
};
};
struct address_space *mapping;
pgoff_t index;
void *private;
atomic_t _mapcount;
atomic_t _refcount;
#ifdef CONFIG_MEMCG;
unsigned long memcg_data;
#endif;
};

with the source code changes being merely:

@@ -227,6 +227,7 @@ struct page {
* struct folio - Represents a contiguous set of bytes.
* @flags: Identical to the page flags.
* @lru: Least Recently Used list; tracks how recently this folio was used.
+ * @mlock_count: Number of times any page in this folio is mlocked.
* @mapping: The file this page belongs to, or refers to the anon_vma for
* anonymous memory.
* @index: Offset within the file, in units of pages. For anonymous memory,
@@ -256,7 +257,9 @@ struct folio {
union {
struct list_head lru;
struct {
+ /* private: */
void *__filler;
+ /* public: */
unsigned int mlock_count;
};
};
Various steps in between are possible, such as removing the anonymous
struct from the documentation, but leaving the union. We could also
choose to document __filler, but that seems like a poor choice to me.

Anyway, that's all arguable and not really too important. My mild
preference is for the private/public markers around __filler alone,
but I'll happily abide by the preferences of others.

The important part is what mlock_count really means. We can be
reasonably verbose here (two or three lines of text, I'd suggest).

\
 
 \ /
  Last update: 2022-03-29 01:09    [W:0.060 / U:1.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site