Re: [PATCH RFC 2/3] mm: Add mempolicy support to the filemap layer

[Shivank Garg <shivankg@amd.com>]

Hello Matthew,

Thank you for the review comments.

On 9/17/2024 3:12 AM, Matthew Wilcox wrote:
> On Mon, Sep 16, 2024 at 04:57:42PM +0000, Shivank Garg wrote:

>> +static inline struct folio *filemap_grab_folio_mpol(struct address_space *mapping,
>> +					pgoff_t index, struct mempolicy *mpol)
>> +{
>> +	return __filemap_get_folio_mpol(mapping, index,
>> +			FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
>> +			mapping_gfp_mask(mapping), mpol);
>> +}
> 
> This should be conditional on CONFIG_NUMA, just like 
> filemap_alloc_folio_mpol_noprof() above.

+#ifdef CONFIG_NUMA
 static inline struct folio *filemap_grab_folio_mpol(struct address_space *mapping,
                                        pgoff_t index, struct mempolicy *mpol)
 {
@@ -739,6 +742,13 @@ static inline struct folio *filemap_grab_folio_mpol(struct address_space *mappin
                        FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
                        mapping_gfp_mask(mapping), mpol);
 }
+#else
+static inline struct folio *filemap_grab_folio_mpol(struct address_space *mapping,
+                                       pgoff_t index, struct mempolicy *mpol)
+{
+       return filemap_grab_folio(mapping, index);
+}
+#endif /* CONFIG_NUMA */

> 
>> @@ -1947,7 +1959,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>>  			err = -ENOMEM;
>>  			if (order > 0)
>>  				alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
>> -			folio = filemap_alloc_folio(alloc_gfp, order);
>> +			folio = filemap_alloc_folio_mpol_noprof(alloc_gfp, order, mpol);
> 
> Why use the _noprof variant here?

I've defined the filemap_alloc_folio_mpol variant for using here:
+#define filemap_alloc_folio_mpol(...)                          \
+       alloc_hooks(filemap_alloc_folio_mpol_noprof(__VA_ARGS__))

+++ b/mm/filemap.c
@@ -1959,7 +1959,7 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping, pgoff_t in
                        err = -ENOMEM;
                        if (order > 0)
                                alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
-                       folio = filemap_alloc_folio_mpol_noprof(alloc_gfp, order, mpol);
+                       folio = filemap_alloc_folio_mpol(alloc_gfp, order, mpol);
                        if (!folio)
   

> 
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 9e9450433fcc..88da732cf2be 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -2281,6 +2281,7 @@ struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>>  	return page_rmappable_folio(alloc_pages_mpol_noprof(gfp | __GFP_COMP,
>>  							order, pol, ilx, nid));
>>  }
>> +EXPORT_SYMBOL(folio_alloc_mpol_noprof);
> 
> Why does this need to be exported?  What module will use itI've removed this EXPORT.

Thank you for the suggestion.
I overlooked those details and will post the replied changes in next version of this patchset.

Best Regards,
Shivank