Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
From: John Garry <john.g.garry@oracle.com>
To: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
Cc: axboe@kernel.dk, brauner@kernel.org, djwong@kernel.org,
	viro@zeniv.linux.org.uk, jack@suse.cz, akpm@linux-foundation.org,
	willy@infradead.org, dchinner@redhat.com, tytso@mit.edu,
	hch@lst.de, martin.petersen@oracle.com, nilay@linux.ibm.com,
	ritesh.list@gmail.com, mcgrof@kernel.org,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, ojaswin@linux.ibm.com, p.raghav@samsung.com,
	jbongio@google.com, okiselev@amazon.com
Subject: Re: [PATCH RFC 2/7] filemap: Change mapping_set_folio_min_order() -> mapping_set_folio_orders()
Date: Fri, 26 Apr 2024 09:02:31 +0100	[thread overview]
Message-ID: <db87a92d-9813-4fdd-bea5-e3d8a6838b30@oracle.com> (raw)
In-Reply-To: <20240425144741.houv6uoflhwmcc2u@quentin>

On 25/04/2024 15:47, Pankaj Raghav (Samsung) wrote:
> On Mon, Apr 22, 2024 at 02:39:18PM +0000, John Garry wrote:
>> Borrowed from:
>>
>> https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20240213093713.1753368-2-kernel@pankajraghav.com/__;!!ACWV5N9M2RV99hQ!LvajFab0xQx8oBWDlDtVY8duiLDjOKX91G4YqadoCu6gqatA2H0FzBUvdSC69dqXNoe2QvStSwrxIZ142MXOKk8$
>> (credit given in due course)
>>
>> We will need to be able to only use a single folio order for buffered
>> atomic writes, so allow the mapping folio order min and max be set.
> 
>>
>> We still have the restriction of not being able to support order-1
>> folios - it will be required to lift this limit at some stage.
> 
> This is already supported upstream for file-backed folios:
> commit: 8897277acfef7f70fdecc054073bea2542fc7a1b

ok

> 
>> index fc8eb9c94e9c..c22455fa28a1 100644
>> --- a/include/linux/pagemap.h
>> +++ b/include/linux/pagemap.h
>> @@ -363,9 +363,10 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
>>   #endif
>>   
>>   /*
>> - * mapping_set_folio_min_order() - Set the minimum folio order
>> + * mapping_set_folio_orders() - Set the minimum and max folio order
> 
> In the new series (sorry forgot to CC you),

no worries, I saw it

> I added a new helper called
> mapping_set_folio_order_range() which does something similar to avoid
> confusion based on willy's suggestion:
> https://urldefense.com/v3/__https://lore.kernel.org/linux-xfs/20240425113746.335530-3-kernel@pankajraghav.com/__;!!ACWV5N9M2RV99hQ!LvajFab0xQx8oBWDlDtVY8duiLDjOKX91G4YqadoCu6gqatA2H0FzBUvdSC69dqXNoe2QvStSwrxIZ14opzAoso$
> 

Fine, I can include that

> mapping_set_folio_min_order() also sets max folio order to be
> MAX_PAGECACHE_ORDER order anyway. So no need of explicitly calling it
> here?
> 

Here mapping_set_folio_min_order() is being replaced with 
mapping_set_folio_order_range(), so not sure why you mention that. 
Regardless, I'll use your mapping_set_folio_order_range().

>>   /**
>> @@ -400,7 +406,7 @@ static inline void mapping_set_folio_min_order(struct address_space *mapping,
>>    */
>>   static inline void mapping_set_large_folios(struct address_space *mapping)
>>   {
>> -	mapping_set_folio_min_order(mapping, 0);
>> +	mapping_set_folio_orders(mapping, 0, MAX_PAGECACHE_ORDER);
>>   }
>>   
>>   static inline unsigned int mapping_max_folio_order(struct address_space *mapping)
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index d81530b0aac0..d5effe50ddcb 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -1898,9 +1898,15 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>>   no_page:
>>   	if (!folio && (fgp_flags & FGP_CREAT)) {
>>   		unsigned int min_order = mapping_min_folio_order(mapping);
>> -		unsigned int order = max(min_order, FGF_GET_ORDER(fgp_flags));
>> +		unsigned int max_order = mapping_max_folio_order(mapping);
>> +		unsigned int order = FGF_GET_ORDER(fgp_flags);
>>   		int err;
>>   
>> +		if (order > max_order)
>> +			order = max_order;
>> +		else if (order < min_order)
>> +			order = max_order;
> 
> order = min_order; ?

right

Thanks,
John

  reply	other threads:[~2024-04-26  8:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-22 14:39 [PATCH RFC 0/7] buffered block atomic writes John Garry
2024-04-22 14:39 ` [PATCH RFC 1/7] fs: Rename STATX{_ATTR}_WRITE_ATOMIC -> STATX{_ATTR}_WRITE_ATOMIC_DIO John Garry
2024-04-22 14:39 ` [PATCH RFC 2/7] filemap: Change mapping_set_folio_min_order() -> mapping_set_folio_orders() John Garry
2024-04-25 14:47   ` Pankaj Raghav (Samsung)
2024-04-26  8:02     ` John Garry [this message]
2024-04-22 14:39 ` [PATCH RFC 3/7] mm: Add PG_atomic John Garry
2024-04-22 14:39 ` [PATCH RFC 4/7] fs: Add initial buffered atomic write support info to statx John Garry
2024-04-22 14:39 ` [PATCH RFC 5/7] fs: iomap: buffered atomic write support John Garry
2024-04-22 15:03   ` Matthew Wilcox
2024-04-22 16:02     ` John Garry
2024-04-22 14:39 ` [PATCH RFC 6/7] fs: xfs: buffered atomic writes statx support John Garry
2024-04-22 14:39 ` [PATCH RFC 7/7] fs: xfs: Enable buffered atomic writes John Garry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=db87a92d-9813-4fdd-bea5-e3d8a6838b30@oracle.com \
    --to=john.g.garry@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=jbongio@google.com \
    --cc=kernel@pankajraghav.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mcgrof@kernel.org \
    --cc=nilay@linux.ibm.com \
    --cc=ojaswin@linux.ibm.com \
    --cc=okiselev@amazon.com \
    --cc=p.raghav@samsung.com \
    --cc=ritesh.list@gmail.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).