[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <001801d5c743$c9c058e0$5d410aa0$@samsung.com>
Date: Fri, 10 Jan 2020 08:23:23 +0900
From: "Namjae Jeon" <namjae.jeon@...sung.com>
To: "'Christoph Hellwig'" <hch@....de>
Cc: <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
<gregkh@...uxfoundation.org>, <valdis.kletnieks@...edu>,
<sj1557.seo@...sung.com>, <linkinjeon@...il.com>,
<pali.rohar@...il.com>
Subject: RE: [PATCH v9 03/13] exfat: add inode operations
> > +#include "exfat_fs.h"
> > +
> > +/* 2-level option flag */
> > +enum {
> > + BMAP_NOT_CREATE,
> > + BMAP_ADD_CLUSTER,
> > +};
>
> I looked at how this flag is used and found the get_block code a little
> confusing. Let me know what you think of the following untested patch to
> streamline that area:
I will check and test this patch.
Thanks!
>
>
> diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c index
> c2b04537cb24..ccf9700c6a55 100644
> --- a/fs/exfat/inode.c
> +++ b/fs/exfat/inode.c
> @@ -17,12 +17,6 @@
> #include "exfat_raw.h"
> #include "exfat_fs.h"
>
> -/* 2-level option flag */
> -enum {
> - BMAP_NOT_CREATE,
> - BMAP_ADD_CLUSTER,
> -};
> -
> static int __exfat_write_inode(struct inode *inode, int sync) {
> int ret = -EIO;
> @@ -298,109 +292,91 @@ static int exfat_map_cluster(struct inode *inode,
> unsigned int clu_offset,
> return 0;
> }
>
> -static int exfat_bmap(struct inode *inode, sector_t sector, sector_t
> *phys,
> - unsigned long *mapped_blocks, int *create)
> +static int exfat_map_new_buffer(struct exfat_inode_info *ei,
> + struct buffer_head *bh, loff_t pos)
> {
> - struct super_block *sb = inode->i_sb;
> - struct exfat_sb_info *sbi = EXFAT_SB(sb);
> - sector_t last_block;
> - unsigned int cluster, clu_offset, sec_offset;
> - int err = 0;
> -
> - *phys = 0;
> - *mapped_blocks = 0;
> -
> - last_block = EXFAT_B_TO_BLK_ROUND_UP(i_size_read(inode), sb);
> - if (sector >= last_block && *create == BMAP_NOT_CREATE)
> - return 0;
> -
> - /* Is this block already allocated? */
> - clu_offset = sector >> sbi->sect_per_clus_bits; /* cluster offset
> */
> -
> - err = exfat_map_cluster(inode, clu_offset, &cluster,
> - *create & BMAP_ADD_CLUSTER);
> - if (err) {
> - if (err != -ENOSPC)
> - return -EIO;
> - return err;
> - }
> -
> - if (cluster != EXFAT_EOF_CLUSTER) {
> - /* sector offset in cluster */
> - sec_offset = sector & (sbi->sect_per_clus - 1);
> -
> - *phys = exfat_cluster_to_sector(sbi, cluster) + sec_offset;
> - *mapped_blocks = sbi->sect_per_clus - sec_offset;
> - }
> + if (buffer_delay(bh) && pos > ei->i_size_aligned)
> + return -EIO;
> + set_buffer_new(bh);
>
> - if (sector < last_block)
> - *create = BMAP_NOT_CREATE;
> + /*
> + * Adjust i_size_aligned if i_size_ondisk is bigger than it.
> + * (i.e. non-DA)
> + */
> + if (ei->i_size_ondisk > ei->i_size_aligned)
> + ei->i_size_aligned = ei->i_size_ondisk;
> return 0;
> }
>
> static int exfat_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create) {
> + struct exfat_inode_info *ei = EXFAT_I(inode);
> struct super_block *sb = inode->i_sb;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> unsigned long max_blocks = bh_result->b_size >> inode->i_blkbits;
> int err = 0;
> - unsigned long mapped_blocks;
> - sector_t phys;
> + unsigned long mapped_blocks = 0;
> + unsigned int cluster, sec_offset;
> + sector_t last_block;
> + sector_t phys = 0;
> loff_t pos;
> - int bmap_create = create ? BMAP_ADD_CLUSTER : BMAP_NOT_CREATE;
> +
> + mutex_lock(&sbi->s_lock);
> + last_block = EXFAT_B_TO_BLK_ROUND_UP(i_size_read(inode), sb);
> + if (iblock >= last_block && !create)
> + goto done;
>
> - mutex_lock(&EXFAT_SB(sb)->s_lock);
> - err = exfat_bmap(inode, iblock, &phys, &mapped_blocks,
> &bmap_create);
> + /* Is this block already allocated? */
> + err = exfat_map_cluster(inode, iblock >> sbi->sect_per_clus_bits,
> + &cluster, create);
> if (err) {
> - if (err != -ENOSPC)
> - exfat_fs_error_ratelimit(sb,
> - "failed to bmap (inode : %p iblock : %llu,
> err : %d)",
> - inode, (unsigned long long)iblock, err);
> + if (err == -ENOSPC)
> + goto unlock_ret;
> +
> + exfat_fs_error_ratelimit(sb,
> + "failed to bmap (inode : %p iblock : %llu, err :
%d)",
> + inode, (unsigned long long)iblock, err);
> goto unlock_ret;
> }
>
> - if (phys) {
> - max_blocks = min(mapped_blocks, max_blocks);
> -
> - /* Treat newly added block / cluster */
> - if (bmap_create || buffer_delay(bh_result)) {
> - /* Update i_size_ondisk */
> - pos = EXFAT_BLK_TO_B((iblock + 1), sb);
> - if (EXFAT_I(inode)->i_size_ondisk < pos)
> - EXFAT_I(inode)->i_size_ondisk = pos;
> -
> - if (bmap_create) {
> - if (buffer_delay(bh_result) &&
> - pos > EXFAT_I(inode)->i_size_aligned) {
> - exfat_fs_error(sb,
> - "requested for bmap out of
> range(pos : (%llu) > i_size_aligned(%llu)\n",
> - pos,
> -
EXFAT_I(inode)->i_size_aligned);
> - err = -EIO;
> - goto unlock_ret;
> - }
> - set_buffer_new(bh_result);
> -
> - /*
> - * adjust i_size_aligned if i_size_ondisk is
> - * bigger than it. (i.e. non-DA)
> - */
> - if (EXFAT_I(inode)->i_size_ondisk >
> - EXFAT_I(inode)->i_size_aligned) {
> - EXFAT_I(inode)->i_size_aligned =
> -
EXFAT_I(inode)->i_size_ondisk;
> - }
> - }
> + if (cluster == EXFAT_EOF_CLUSTER)
> + goto done;
> +
> + /* sector offset in cluster */
> + sec_offset = iblock & (sbi->sect_per_clus - 1);
> +
> + phys = exfat_cluster_to_sector(sbi, cluster) + sec_offset;
> + mapped_blocks = sbi->sect_per_clus - sec_offset;
> + max_blocks = min(mapped_blocks, max_blocks);
>
> - if (buffer_delay(bh_result))
> - clear_buffer_delay(bh_result);
> + /* Treat newly added block / cluster */
> + if (iblock < last_block)
> + create = 0;
> +
> + if (create || buffer_delay(bh_result)) {
> + pos = EXFAT_BLK_TO_B((iblock + 1), sb);
> + if (ei->i_size_ondisk < pos)
> + ei->i_size_ondisk = pos;
> + }
> +
> + if (create) {
> + err = exfat_map_new_buffer(ei, bh_result, pos);
> + if (err) {
> + exfat_fs_error(sb,
> + "requested for bmap out of range(pos :
(%llu) >
> i_size_aligned(%llu)\n",
> + pos, ei->i_size_aligned);
> + goto unlock_ret;
> }
> - map_bh(bh_result, sb, phys);
> }
>
> + if (buffer_delay(bh_result))
> + clear_buffer_delay(bh_result);
> + map_bh(bh_result, sb, phys);
> +done:
> bh_result->b_size = EXFAT_BLK_TO_B(max_blocks, sb);
> unlock_ret:
> - mutex_unlock(&EXFAT_SB(sb)->s_lock);
> + mutex_unlock(&sbi->s_lock);
> return err;
> }
>
Powered by blists - more mailing lists