Change the signature of ufs_get_page() in order to prepare this function
to the conversion to the use of kmap_local_page(). Change also those call
sites which are required to conform its invocations to the new
signature.
Cc: Ira Weiny <ira.weiny@intel.com>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
fs/ufs/dir.c | 49 +++++++++++++++++++++----------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c
index 69f78583c9c1..9fa86614d2d1 100644
--- a/fs/ufs/dir.c
+++ b/fs/ufs/dir.c
@@ -185,7 +185,7 @@ static bool ufs_check_page(struct page *page)
return false;
}
-static struct page *ufs_get_page(struct inode *dir, unsigned long n)
+static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **p)
{
struct address_space *mapping = dir->i_mapping;
struct page *page = read_mapping_page(mapping, n, NULL);
@@ -195,8 +195,10 @@ static struct page *ufs_get_page(struct inode *dir, unsigned long n)
if (!ufs_check_page(page))
goto fail;
}
+ *p = page;
+ return page_address(page);
}
- return page;
+ return ERR_CAST(page);
fail:
ufs_put_page(page);
@@ -227,15 +229,12 @@ ufs_next_entry(struct super_block *sb, struct ufs_dir_entry *p)
struct ufs_dir_entry *ufs_dotdot(struct inode *dir, struct page **p)
{
- struct page *page = ufs_get_page(dir, 0);
- struct ufs_dir_entry *de = NULL;
+ struct ufs_dir_entry *de = ufs_get_page(dir, 0, p);
- if (!IS_ERR(page)) {
- de = ufs_next_entry(dir->i_sb,
- (struct ufs_dir_entry *)page_address(page));
- *p = page;
- }
- return de;
+ if (!IS_ERR(de))
+ return ufs_next_entry(dir->i_sb, de);
+ else
+ return NULL;
}
/*
@@ -273,11 +272,10 @@ struct ufs_dir_entry *ufs_find_entry(struct inode *dir, const struct qstr *qstr,
start = 0;
n = start;
do {
- char *kaddr;
- page = ufs_get_page(dir, n);
- if (!IS_ERR(page)) {
- kaddr = page_address(page);
- de = (struct ufs_dir_entry *) kaddr;
+ char *kaddr = ufs_get_page(dir, n, &page);
+
+ if (!IS_ERR(kaddr)) {
+ de = (struct ufs_dir_entry *)kaddr;
kaddr += ufs_last_byte(dir, n) - reclen;
while ((char *) de <= kaddr) {
if (ufs_match(sb, namelen, name, de))
@@ -328,12 +326,10 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode)
for (n = 0; n <= npages; n++) {
char *dir_end;
- page = ufs_get_page(dir, n);
- err = PTR_ERR(page);
- if (IS_ERR(page))
- goto out;
+ kaddr = ufs_get_page(dir, n, &page);
+ if (IS_ERR(kaddr))
+ return PTR_ERR(kaddr);
lock_page(page);
- kaddr = page_address(page);
dir_end = kaddr + ufs_last_byte(dir, n);
de = (struct ufs_dir_entry *)kaddr;
kaddr += PAGE_SIZE - reclen;
@@ -395,7 +391,6 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode)
/* OFFSET_CACHE */
out_put:
ufs_put_page(page);
-out:
return err;
out_unlock:
unlock_page(page);
@@ -429,6 +424,7 @@ ufs_readdir(struct file *file, struct dir_context *ctx)
unsigned chunk_mask = ~(UFS_SB(sb)->s_uspi->s_dirblksize - 1);
bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
unsigned flags = UFS_SB(sb)->s_flags;
+ struct page *page;
UFSD("BEGIN\n");
@@ -439,16 +435,14 @@ ufs_readdir(struct file *file, struct dir_context *ctx)
char *kaddr, *limit;
struct ufs_dir_entry *de;
- struct page *page = ufs_get_page(inode, n);
-
- if (IS_ERR(page)) {
+ kaddr = ufs_get_page(inode, n, &page);
+ if (IS_ERR(kaddr)) {
ufs_error(sb, __func__,
"bad page in #%lu",
inode->i_ino);
ctx->pos += PAGE_SIZE - offset;
return -EIO;
}
- kaddr = page_address(page);
if (unlikely(need_revalidate)) {
if (offset) {
offset = ufs_validate_entry(sb, kaddr, offset, chunk_mask);
@@ -595,12 +589,11 @@ int ufs_empty_dir(struct inode * inode)
for (i = 0; i < npages; i++) {
char *kaddr;
struct ufs_dir_entry *de;
- page = ufs_get_page(inode, i);
- if (IS_ERR(page))
+ kaddr = ufs_get_page(inode, i, &page);
+ if (IS_ERR(kaddr))
continue;
- kaddr = page_address(page);
de = (struct ufs_dir_entry *)kaddr;
kaddr += ufs_last_byte(inode, i) - UFS_DIR_REC_LEN(1);
--
2.39.0On Wed, Dec 21, 2022 at 06:28:01PM +0100, Fabio M. De Francesco wrote:
> Change the signature of ufs_get_page() in order to prepare this function
> to the conversion to the use of kmap_local_page(). Change also those call
> sites which are required to conform its invocations to the new
> signature.
>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> fs/ufs/dir.c | 49 +++++++++++++++++++++----------------------------
> 1 file changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c
> index 69f78583c9c1..9fa86614d2d1 100644
> --- a/fs/ufs/dir.c
> +++ b/fs/ufs/dir.c
> @@ -185,7 +185,7 @@ static bool ufs_check_page(struct page *page)
> return false;
> }
>
> -static struct page *ufs_get_page(struct inode *dir, unsigned long n)
> +static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **p)
> {
> struct address_space *mapping = dir->i_mapping;
> struct page *page = read_mapping_page(mapping, n, NULL);
> @@ -195,8 +195,10 @@ static struct page *ufs_get_page(struct inode *dir, unsigned long n)
> if (!ufs_check_page(page))
> goto fail;
> }
> + *p = page;
> + return page_address(page);
> }
> - return page;
> + return ERR_CAST(page);
>
> fail:
> ufs_put_page(page);
> @@ -227,15 +229,12 @@ ufs_next_entry(struct super_block *sb, struct ufs_dir_entry *p)
>
> struct ufs_dir_entry *ufs_dotdot(struct inode *dir, struct page **p)
> {
> - struct page *page = ufs_get_page(dir, 0);
> - struct ufs_dir_entry *de = NULL;
> + struct ufs_dir_entry *de = ufs_get_page(dir, 0, p);
I don't know why but ufs_get_page() returning an address read really odd to me.
But rolling around my head alternative names nothing seems better than this.
>
> - if (!IS_ERR(page)) {
> - de = ufs_next_entry(dir->i_sb,
> - (struct ufs_dir_entry *)page_address(page));
> - *p = page;
> - }
> - return de;
> + if (!IS_ERR(de))
> + return ufs_next_entry(dir->i_sb, de);
> + else
> + return NULL;
> }
>
> /*
> @@ -273,11 +272,10 @@ struct ufs_dir_entry *ufs_find_entry(struct inode *dir, const struct qstr *qstr,
> start = 0;
> n = start;
> do {
> - char *kaddr;
> - page = ufs_get_page(dir, n);
> - if (!IS_ERR(page)) {
> - kaddr = page_address(page);
> - de = (struct ufs_dir_entry *) kaddr;
> + char *kaddr = ufs_get_page(dir, n, &page);
> +
> + if (!IS_ERR(kaddr)) {
> + de = (struct ufs_dir_entry *)kaddr;
> kaddr += ufs_last_byte(dir, n) - reclen;
> while ((char *) de <= kaddr) {
> if (ufs_match(sb, namelen, name, de))
> @@ -328,12 +326,10 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode)
> for (n = 0; n <= npages; n++) {
> char *dir_end;
>
> - page = ufs_get_page(dir, n);
> - err = PTR_ERR(page);
> - if (IS_ERR(page))
> - goto out;
> + kaddr = ufs_get_page(dir, n, &page);
> + if (IS_ERR(kaddr))
> + return PTR_ERR(kaddr);
> lock_page(page);
> - kaddr = page_address(page);
> dir_end = kaddr + ufs_last_byte(dir, n);
> de = (struct ufs_dir_entry *)kaddr;
> kaddr += PAGE_SIZE - reclen;
> @@ -395,7 +391,6 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode)
> /* OFFSET_CACHE */
> out_put:
> ufs_put_page(page);
> -out:
> return err;
> out_unlock:
> unlock_page(page);
> @@ -429,6 +424,7 @@ ufs_readdir(struct file *file, struct dir_context *ctx)
> unsigned chunk_mask = ~(UFS_SB(sb)->s_uspi->s_dirblksize - 1);
> bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
> unsigned flags = UFS_SB(sb)->s_flags;
> + struct page *page;
NIT: Does page now leave the scope of the for loop?
>
> UFSD("BEGIN\n");
>
> @@ -439,16 +435,14 @@ ufs_readdir(struct file *file, struct dir_context *ctx)
> char *kaddr, *limit;
> struct ufs_dir_entry *de;
Couldn't that be declared here?
Regardless I don't think this is broken.
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
>
> - struct page *page = ufs_get_page(inode, n);
> -
> - if (IS_ERR(page)) {
> + kaddr = ufs_get_page(inode, n, &page);
> + if (IS_ERR(kaddr)) {
> ufs_error(sb, __func__,
> "bad page in #%lu",
> inode->i_ino);
> ctx->pos += PAGE_SIZE - offset;
> return -EIO;
> }
> - kaddr = page_address(page);
> if (unlikely(need_revalidate)) {
> if (offset) {
> offset = ufs_validate_entry(sb, kaddr, offset, chunk_mask);
> @@ -595,12 +589,11 @@ int ufs_empty_dir(struct inode * inode)
> for (i = 0; i < npages; i++) {
> char *kaddr;
> struct ufs_dir_entry *de;
> - page = ufs_get_page(inode, i);
>
> - if (IS_ERR(page))
> + kaddr = ufs_get_page(inode, i, &page);
> + if (IS_ERR(kaddr))
> continue;
>
> - kaddr = page_address(page);
> de = (struct ufs_dir_entry *)kaddr;
> kaddr += ufs_last_byte(inode, i) - UFS_DIR_REC_LEN(1);
>
> --
> 2.39.0
>
On giovedì 22 dicembre 2022 06:13:43 CET Ira Weiny wrote:
> On Wed, Dec 21, 2022 at 06:28:01PM +0100, Fabio M. De Francesco wrote:
> > Change the signature of ufs_get_page() in order to prepare this function
> > to the conversion to the use of kmap_local_page(). Change also those call
> > sites which are required to conform its invocations to the new
> > signature.
> >
> > Cc: Ira Weiny <ira.weiny@intel.com>
> > Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> >
> > fs/ufs/dir.c | 49 +++++++++++++++++++++----------------------------
> > 1 file changed, 21 insertions(+), 28 deletions(-)
> >
> > diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c
> > index 69f78583c9c1..9fa86614d2d1 100644
> > --- a/fs/ufs/dir.c
> > +++ b/fs/ufs/dir.c
> > @@ -185,7 +185,7 @@ static bool ufs_check_page(struct page *page)
> >
> > return false;
> >
> > }
> >
> > -static struct page *ufs_get_page(struct inode *dir, unsigned long n)
> > +static void *ufs_get_page(struct inode *dir, unsigned long n, struct page
> > **p)>
> > {
> >
> > struct address_space *mapping = dir->i_mapping;
> > struct page *page = read_mapping_page(mapping, n, NULL);
> >
> > @@ -195,8 +195,10 @@ static struct page *ufs_get_page(struct inode *dir,
> > unsigned long n)>
> > if (!ufs_check_page(page))
> >
> > goto fail;
> >
> > }
> >
> > + *p = page;
> > + return page_address(page);
> >
> > }
> >
> > - return page;
> > + return ERR_CAST(page);
> >
> > fail:
> > ufs_put_page(page);
> >
> > @@ -227,15 +229,12 @@ ufs_next_entry(struct super_block *sb, struct
> > ufs_dir_entry *p)>
> > struct ufs_dir_entry *ufs_dotdot(struct inode *dir, struct page **p)
> > {
> >
> > - struct page *page = ufs_get_page(dir, 0);
> > - struct ufs_dir_entry *de = NULL;
> > + struct ufs_dir_entry *de = ufs_get_page(dir, 0, p);
>
> I don't know why but ufs_get_page() returning an address read really odd to
> me. But rolling around my head alternative names nothing seems better than
> this.
ufs_get_kaddr()?
> > - if (!IS_ERR(page)) {
> > - de = ufs_next_entry(dir->i_sb,
> > - (struct ufs_dir_entry
*)page_address(page));
> > - *p = page;
> > - }
> > - return de;
> > + if (!IS_ERR(de))
> > + return ufs_next_entry(dir->i_sb, de);
> > + else
> > + return NULL;
> >
> > }
> >
> > /*
> >
> > @@ -273,11 +272,10 @@ struct ufs_dir_entry *ufs_find_entry(struct inode
> > *dir, const struct qstr *qstr,>
> > start = 0;
> >
> > n = start;
> > do {
> >
> > - char *kaddr;
> > - page = ufs_get_page(dir, n);
> > - if (!IS_ERR(page)) {
> > - kaddr = page_address(page);
> > - de = (struct ufs_dir_entry *) kaddr;
> > + char *kaddr = ufs_get_page(dir, n, &page);
> > +
> > + if (!IS_ERR(kaddr)) {
> > + de = (struct ufs_dir_entry *)kaddr;
> >
> > kaddr += ufs_last_byte(dir, n) - reclen;
> > while ((char *) de <= kaddr) {
> >
> > if (ufs_match(sb, namelen, name, de))
> >
> > @@ -328,12 +326,10 @@ int ufs_add_link(struct dentry *dentry, struct inode
> > *inode)>
> > for (n = 0; n <= npages; n++) {
> >
> > char *dir_end;
> >
> > - page = ufs_get_page(dir, n);
> > - err = PTR_ERR(page);
> > - if (IS_ERR(page))
> > - goto out;
> > + kaddr = ufs_get_page(dir, n, &page);
> > + if (IS_ERR(kaddr))
> > + return PTR_ERR(kaddr);
> >
> > lock_page(page);
> >
> > - kaddr = page_address(page);
> >
> > dir_end = kaddr + ufs_last_byte(dir, n);
> > de = (struct ufs_dir_entry *)kaddr;
> > kaddr += PAGE_SIZE - reclen;
> >
> > @@ -395,7 +391,6 @@ int ufs_add_link(struct dentry *dentry, struct inode
> > *inode)>
> > /* OFFSET_CACHE */
> >
> > out_put:
> > ufs_put_page(page);
> >
> > -out:
> > return err;
> >
> > out_unlock:
> > unlock_page(page);
> >
> > @@ -429,6 +424,7 @@ ufs_readdir(struct file *file, struct dir_context
*ctx)
> >
> > unsigned chunk_mask = ~(UFS_SB(sb)->s_uspi->s_dirblksize - 1);
> > bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
> > unsigned flags = UFS_SB(sb)->s_flags;
> >
> > + struct page *page;
>
> NIT: Does page now leave the scope of the for loop?
>
Strange...
I can't say why I did so.
> > UFSD("BEGIN\n");
> >
> > @@ -439,16 +435,14 @@ ufs_readdir(struct file *file, struct dir_context
> > *ctx)
> >
> > char *kaddr, *limit;
> > struct ufs_dir_entry *de;
>
> Couldn't that be declared here?
Yes, it could :-)
> Regardless I don't think this is broken.
Since I have to submit a new version of this series, there's no problem moving
the declaration of "page" back into the loop.
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks,
Fabio
>
> > - struct page *page = ufs_get_page(inode, n);
> > -
> > - if (IS_ERR(page)) {
> > + kaddr = ufs_get_page(inode, n, &page);
> > + if (IS_ERR(kaddr)) {
> >
> > ufs_error(sb, __func__,
> >
> > "bad page in #%lu",
> > inode->i_ino);
> >
> > ctx->pos += PAGE_SIZE - offset;
> > return -EIO;
> >
> > }
> >
> > - kaddr = page_address(page);
> >
> > if (unlikely(need_revalidate)) {
> >
> > if (offset) {
> >
> > offset = ufs_validate_entry(sb, kaddr,
offset, chunk_mask);
> >
> > @@ -595,12 +589,11 @@ int ufs_empty_dir(struct inode * inode)
> >
> > for (i = 0; i < npages; i++) {
> >
> > char *kaddr;
> > struct ufs_dir_entry *de;
> >
> > - page = ufs_get_page(inode, i);
> >
> > - if (IS_ERR(page))
> > + kaddr = ufs_get_page(inode, i, &page);
> > + if (IS_ERR(kaddr))
> >
> > continue;
> >
> > - kaddr = page_address(page);
> >
> > de = (struct ufs_dir_entry *)kaddr;
> > kaddr += ufs_last_byte(inode, i) - UFS_DIR_REC_LEN(1);
> >
> > --
> > 2.39.0
© 2016 - 2025 Red Hat, Inc.