Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 6 Jun 2021 15:13:51 +0100
From:   Christoph Hellwig <hch@infradead.org>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Christoph Hellwig <hch@infradead.org>, akpm@linux-foundation.org,
        linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 08/33] mm: Add folio_try_get_rcu
Message-ID: <YLzYH3G3Jg7K2jpE@infradead.org>
References: <20210511214735.1836149-1-willy@infradead.org>
 <20210511214735.1836149-9-willy@infradead.org>
 <YK9VagEO+bKurYlz@infradead.org>
 <YLr9E4tWle9Qa1Xy@casper.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YLr9E4tWle9Qa1Xy@casper.infradead.org>
Precedence: bulk

On Sat, Jun 05, 2021 at 05:26:59AM +0100, Matthew Wilcox wrote:
> On Thu, May 27, 2021 at 09:16:42AM +0100, Christoph Hellwig wrote:
> > On Tue, May 11, 2021 at 10:47:10PM +0100, Matthew Wilcox (Oracle) wrote:
> > > +static inline bool folio_ref_try_add_rcu(struct folio *folio, int count)
> > 
> > Should this have a __ prefix and/or a don't use direct comment?
> 
> I think it will get used directly ... its page counterpart is:
> 
> mm/gup.c:       if (unlikely(!page_cache_add_speculative(head, refs)))
> 
> I deliberately left kernel-doc off this function so it's not described,
> but described folio_try_get_rcu() in excruciating detail.  I hope that's
> enough.  There's no comment on page_cache_add_speculative() today, so
> again, we're status quo.

Ok.  Seems a little weird, but so does much in this area.

> > Is this really a good place for the comment?  I'd expect it either near
> > a relevant function or at the top of a file.
> 
> It's right before mapping_get_entry() which is the main lookup function
> for the page cache, so I think it meets your first criteria?

I guess it does, yes.