linux-kernel - Re: [PATCH v8 01/10] fs: Allow fine-grained control of folio sizes

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240704212830.xtakuw57wonas42u@quentin>
Date: Thu, 4 Jul 2024 21:28:30 +0000
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Ryan Roberts <ryan.roberts@....com>, david@...morbit.com,
	chandan.babu@...cle.com, djwong@...nel.org, brauner@...nel.org,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	yang@...amperecomputing.com, linux-mm@...ck.org,
	john.g.garry@...cle.com, linux-fsdevel@...r.kernel.org,
	hare@...e.de, p.raghav@...sung.com, mcgrof@...nel.org,
	gost.dev@...sung.com, cl@...amperecomputing.com,
	linux-xfs@...r.kernel.org, hch@....de, Zi Yan <zi.yan@...t.com>
Subject: Re: [PATCH v8 01/10] fs: Allow fine-grained control of folio sizes

On Thu, Jul 04, 2024 at 04:20:13PM +0100, Matthew Wilcox wrote:
> On Thu, Jul 04, 2024 at 01:23:20PM +0100, Ryan Roberts wrote:
> > > -	AS_LARGE_FOLIO_SUPPORT = 6,
> > 
> > nit: this removed enum is still referenced in a comment further down the file.
Good catch.
> 
> Thanks.  Pankaj, let me know if you want me to send you a patch or if
> you'll do it directly.
Yes, I will fold the changes.
> 
> > > +static inline void mapping_set_folio_order_range(struct address_space *mapping,
> > > +						 unsigned int min,
> > > +						 unsigned int max)
> > > +{
> > > +	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> > > +		return;
> > > +
> > > +	if (min > MAX_PAGECACHE_ORDER)
> > > +		min = MAX_PAGECACHE_ORDER;
> > > +	if (max > MAX_PAGECACHE_ORDER)
> > > +		max = MAX_PAGECACHE_ORDER;
> > > +	if (max < min)
> > > +		max = min;
> > 
> > It seems strange to silently clamp these? Presumably for the bs>ps usecase,
> > whatever values are passed in are a hard requirement? So wouldn't want them to
> > be silently reduced. (Especially given the recent change to reduce the size of
> > MAX_PAGECACHE_ORDER to less then PMD size in some cases).
> 
> Hm, yes.  We should probably make this return an errno.  Including
> returning an errno for !IS_ENABLED() and min > 0.
> 

Something like this? (I also need to change the xfs_icache.c to
use this return value in the last patch)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 14e1415f7dcf..04916720f807 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -390,28 +390,27 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
  * Context: This should not be called while the inode is active as it
  * is non-atomic.
  */
-static inline void mapping_set_folio_order_range(struct address_space *mapping,
+static inline int mapping_set_folio_order_range(struct address_space *mapping,
                                                 unsigned int min,
                                                 unsigned int max)
 {
        if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
-               return;
+               return -EINVAL;
 
-       if (min > MAX_PAGECACHE_ORDER)
-               min = MAX_PAGECACHE_ORDER;
-       if (max > MAX_PAGECACHE_ORDER)
-               max = MAX_PAGECACHE_ORDER;
+       if (min > MAX_PAGECACHE_ORDER || max > MAX_PAGECACHE_ORDER)
+               return -EINVAL;
        if (max < min)
                max = min;
 
        mapping->flags = (mapping->flags & ~AS_FOLIO_ORDER_MASK) |
                (min << AS_FOLIO_ORDER_MIN) | (max << AS_FOLIO_ORDER_MAX);
+       return 0;
 }
 
-static inline void mapping_set_folio_min_order(struct address_space *mapping,
+static inline int mapping_set_folio_min_order(struct address_space *mapping,
                                               unsigned int min)
 {
-       mapping_set_folio_order_range(mapping, min, MAX_PAGECACHE_ORDER);
+       return mapping_set_folio_order_range(mapping, min, MAX_PAGECACHE_ORDER);
 }
 
 
@@ -428,6 +427,10 @@ static inline void mapping_set_folio_min_order(struct address_space *mapping,
  */
 static inline void mapping_set_large_folios(struct address_space *mapping)
 {
+       /*
+        * The return value can be safely ignored because this range
+        * will always be supported by the page cache.
+        */
        mapping_set_folio_order_range(mapping, 0, MAX_PAGECACHE_ORDER);
 }

 --
 Pankaj