I realised this didn't have a cc to linux-mm.
For that matter, linux-doc might have some opinions.
----- Forwarded message from Matthew Wilcox <willy@xxxxxxxxxxxxx> -----
Date: Tue, 29 Mar 2022 00:04:01 +0100
From: Matthew Wilcox <willy@xxxxxxxxxxxxx>
To: Hugh Dickins <hughd@xxxxxxxxxx>
Cc: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx>, Linux Kernel Mailing List
<linux-kernel@xxxxxxxxxxxxxxx>, Linux Next Mailing List
<linux-next@xxxxxxxxxxxxxxx>
Subject: Re: 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).
----- End forwarded message -----
