* [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
@ 2024-11-15 6:08 Sergey Senozhatsky
2024-11-15 12:07 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Sergey Senozhatsky @ 2024-11-15 6:08 UTC (permalink / raw)
To: stable; +Cc: Jan Kara, linux-kernel, Sergey Senozhatsky, kernel test robot
From: Jan Kara <jack@suse.cz>
[ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
Currently we allocate name buffer in directory iterators (struct
udf_fileident_iter) on stack. These structures are relatively large
(some 360 bytes on 64-bit architectures). For udf_rename() which needs
to keep three of these structures in parallel the stack usage becomes
rather heavy - 1536 bytes in total. Allocate the name buffer in the
iterator from heap to avoid excessive stack usage.
Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
[ senozhatsky: explicitly include slab.h to address build
failure reported by sashal@ ]
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
fs/udf/directory.c | 24 ++++++++++++++++--------
fs/udf/udfdecl.h | 2 +-
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index e97ffae07833..a30898debdd1 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -19,6 +19,7 @@
#include <linux/bio.h>
#include <linux/crc-itu-t.h>
#include <linux/iversion.h>
+#include <linux/slab.h>
static int udf_verify_fi(struct udf_fileident_iter *iter)
{
@@ -248,9 +249,14 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
iter->elen = 0;
iter->epos.bh = NULL;
iter->name = NULL;
+ iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
+ if (!iter->namebuf)
+ return -ENOMEM;
- if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
- return udf_copy_fi(iter);
+ if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
+ err = udf_copy_fi(iter);
+ goto out;
+ }
if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
&iter->eloc, &iter->elen, &iter->loffset) !=
@@ -260,17 +266,17 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
udf_err(dir->i_sb,
"position %llu not allocated in directory (ino %lu)\n",
(unsigned long long)pos, dir->i_ino);
- return -EFSCORRUPTED;
+ err = -EFSCORRUPTED;
+ goto out;
}
err = udf_fiiter_load_bhs(iter);
if (err < 0)
- return err;
+ goto out;
err = udf_copy_fi(iter);
- if (err < 0) {
+out:
+ if (err < 0)
udf_fiiter_release(iter);
- return err;
- }
- return 0;
+ return err;
}
int udf_fiiter_advance(struct udf_fileident_iter *iter)
@@ -307,6 +313,8 @@ void udf_fiiter_release(struct udf_fileident_iter *iter)
brelse(iter->bh[0]);
brelse(iter->bh[1]);
iter->bh[0] = iter->bh[1] = NULL;
+ kfree(iter->namebuf);
+ iter->namebuf = NULL;
}
static void udf_copy_to_bufs(void *buf1, int len1, void *buf2, int len2,
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index f764b4d15094..d35aa42bb577 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -99,7 +99,7 @@ struct udf_fileident_iter {
struct extent_position epos; /* Position after the above extent */
struct fileIdentDesc fi; /* Copied directory entry */
uint8_t *name; /* Pointer to entry name */
- uint8_t namebuf[UDF_NAME_LEN_CS0]; /* Storage for entry name in case
+ uint8_t *namebuf; /* Storage for entry name in case
* the name is split between two blocks
*/
};
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
2024-11-15 6:08 [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap Sergey Senozhatsky
@ 2024-11-15 12:07 ` Jan Kara
2024-11-16 4:04 ` Sergey Senozhatsky
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2024-11-15 12:07 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: stable, Jan Kara, linux-kernel, kernel test robot
On Fri 15-11-24 15:08:48, Sergey Senozhatsky wrote:
> From: Jan Kara <jack@suse.cz>
>
> [ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
>
> Currently we allocate name buffer in directory iterators (struct
> udf_fileident_iter) on stack. These structures are relatively large
> (some 360 bytes on 64-bit architectures). For udf_rename() which needs
> to keep three of these structures in parallel the stack usage becomes
> rather heavy - 1536 bytes in total. Allocate the name buffer in the
> iterator from heap to avoid excessive stack usage.
>
> Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> [ senozhatsky: explicitly include slab.h to address build
> failure reported by sashal@ ]
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Mauke beat you to this, Greg already queued up his patches :)
[1] https://lore.kernel.org/all/20241114212657.306989-1-hauke@hauke-m.de
Honza
> ---
> fs/udf/directory.c | 24 ++++++++++++++++--------
> fs/udf/udfdecl.h | 2 +-
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/fs/udf/directory.c b/fs/udf/directory.c
> index e97ffae07833..a30898debdd1 100644
> --- a/fs/udf/directory.c
> +++ b/fs/udf/directory.c
> @@ -19,6 +19,7 @@
> #include <linux/bio.h>
> #include <linux/crc-itu-t.h>
> #include <linux/iversion.h>
> +#include <linux/slab.h>
>
> static int udf_verify_fi(struct udf_fileident_iter *iter)
> {
> @@ -248,9 +249,14 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
> iter->elen = 0;
> iter->epos.bh = NULL;
> iter->name = NULL;
> + iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
> + if (!iter->namebuf)
> + return -ENOMEM;
>
> - if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
> - return udf_copy_fi(iter);
> + if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
> + err = udf_copy_fi(iter);
> + goto out;
> + }
>
> if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
> &iter->eloc, &iter->elen, &iter->loffset) !=
> @@ -260,17 +266,17 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
> udf_err(dir->i_sb,
> "position %llu not allocated in directory (ino %lu)\n",
> (unsigned long long)pos, dir->i_ino);
> - return -EFSCORRUPTED;
> + err = -EFSCORRUPTED;
> + goto out;
> }
> err = udf_fiiter_load_bhs(iter);
> if (err < 0)
> - return err;
> + goto out;
> err = udf_copy_fi(iter);
> - if (err < 0) {
> +out:
> + if (err < 0)
> udf_fiiter_release(iter);
> - return err;
> - }
> - return 0;
> + return err;
> }
>
> int udf_fiiter_advance(struct udf_fileident_iter *iter)
> @@ -307,6 +313,8 @@ void udf_fiiter_release(struct udf_fileident_iter *iter)
> brelse(iter->bh[0]);
> brelse(iter->bh[1]);
> iter->bh[0] = iter->bh[1] = NULL;
> + kfree(iter->namebuf);
> + iter->namebuf = NULL;
> }
>
> static void udf_copy_to_bufs(void *buf1, int len1, void *buf2, int len2,
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index f764b4d15094..d35aa42bb577 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -99,7 +99,7 @@ struct udf_fileident_iter {
> struct extent_position epos; /* Position after the above extent */
> struct fileIdentDesc fi; /* Copied directory entry */
> uint8_t *name; /* Pointer to entry name */
> - uint8_t namebuf[UDF_NAME_LEN_CS0]; /* Storage for entry name in case
> + uint8_t *namebuf; /* Storage for entry name in case
> * the name is split between two blocks
> */
> };
> --
> 2.47.0.338.g60cca15819-goog
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
2024-11-15 12:07 ` Jan Kara
@ 2024-11-16 4:04 ` Sergey Senozhatsky
0 siblings, 0 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2024-11-16 4:04 UTC (permalink / raw)
To: Jan Kara; +Cc: Sergey Senozhatsky, stable, linux-kernel, kernel test robot
On (24/11/15 13:07), Jan Kara wrote:
> On Fri 15-11-24 15:08:48, Sergey Senozhatsky wrote:
> > From: Jan Kara <jack@suse.cz>
> >
> > [ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
> >
> > Currently we allocate name buffer in directory iterators (struct
> > udf_fileident_iter) on stack. These structures are relatively large
> > (some 360 bytes on 64-bit architectures). For udf_rename() which needs
> > to keep three of these structures in parallel the stack usage becomes
> > rather heavy - 1536 bytes in total. Allocate the name buffer in the
> > iterator from heap to avoid excessive stack usage.
> >
> > Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > [ senozhatsky: explicitly include slab.h to address build
> > failure reported by sashal@ ]
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>
> Mauke beat you to this, Greg already queued up his patches :)
Story of my life :)
> [1] https://lore.kernel.org/all/20241114212657.306989-1-hauke@hauke-m.de
Awesome, thanks everyone!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-16 4:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-15 6:08 [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap Sergey Senozhatsky
2024-11-15 12:07 ` Jan Kara
2024-11-16 4:04 ` Sergey Senozhatsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.