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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e471ad3c-daf1-4a21-b800-9934d1d3d9df@amd.com>
Date: Tue, 17 Sep 2024 18:13:13 +0530
From: Shivank Garg <shivankg@....com>
To: Matthew Wilcox <willy@...radead.org>
Cc: pbonzini@...hat.com, corbet@....net, akpm@...ux-foundation.org,
 acme@...hat.com, namhyung@...nel.org, mpe@...erman.id.au,
 isaku.yamahata@...el.com, joel@....id.au, kvm@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 linux-fsdevel@...r.kernel.org, bharata@....com, nikunj@....com
Subject: Re: [PATCH RFC 2/3] mm: Add mempolicy support to the filemap layer

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