All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Cc: Damien Le Moal <Damien.LeMoal@wdc.com>,
	"linux-f2fs-devel@lists.sourceforge.net"
	<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH] mkfs.f2fs: wipe other FS magics given -f
Date: Mon, 20 Sep 2021 16:26:57 -0700	[thread overview]
Message-ID: <YUkYwQidnXGB9LLk@google.com> (raw)
In-Reply-To: <20210915054532.alaps3utwwei4efo@shindev>

On 09/15, Shinichiro Kawasaki wrote:
> On Sep 10, 2021 / 15:56, Jaegeuk Kim wrote:
> > This patch fixes the below stale magic info.
> > 
> > $ mkfs.btrfs -f test.img
> > btrfs-progs v5.10.1
> > See http://btrfs.wiki.kernel.org for more information.
> > 
> > Label:              (null)
> > UUID:               941d2db7-3ece-4090-8b22-c4ea548b5dae
> > Node size:          16384
> > Sector size:        4096
> > Filesystem size:    1.00GiB
> > Block group profiles:
> >   Data:             single            8.00MiB
> >   Metadata:         DUP              51.19MiB
> >   System:           DUP               8.00MiB
> > SSD detected:       no
> > Incompat features:  extref, skinny-metadata
> > Runtime features:
> > Checksum:           crc32c
> > Number of devices:  1
> > Devices:
> >    ID        SIZE  PATH
> >     1     1.00GiB  test.img
> > 
> > $ hexdump -s 0x10000 -n 128 -C test.img
> > 00010000  81 29 94 0a 00 00 00 00  00 00 00 00 00 00 00 00  |.)..............|
> > 00010010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > 00010020  94 1d 2d b7 3e ce 40 90  8b 22 c4 ea 54 8b 5d ae  |..-.>.@.."..T.].|
> > 00010030  00 00 01 00 00 00 00 00  01 00 00 00 00 00 00 00  |................|
> > 00010040  5f 42 48 52 66 53 5f 4d  05 00 00 00 00 00 00 00  |_BHRfS_M........|
> > 00010050  00 40 d2 01 00 00 00 00  00 40 50 01 00 00 00 00  |.@.......@P.....|
> > 00010060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > 00010070  00 00 00 40 00 00 00 00  00 00 02 00 00 00 00 00  |...@............|
> > 00010080
> > 
> > $ mkfs.f2fs -t 0 -f test.img
> > 
> > 	F2FS-tools: mkfs.f2fs Ver: 1.14.0 (2020-12-28)
> > 
> > Info: Disable heap-based policy
> > Info: Debug level = 0
> > Info: Trim is disabled
> > Info: Segments per section = 1
> > Info: Sections per zone = 1
> > Info: sector size = 512
> > Info: total sectors = 2097152 (1024 MB)
> > Info: zone aligned segment0 blkaddr: 512
> > Info: format version with
> >   "Linux version 5.10.46-4rodete1-amd64 (glinux-team@google.com) (gcc-10 (Debian 10.2.1-6+build2) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2) #1 SMP Debian 5.10.46-4rodete1 (2021-08-20)"
> > Info: Overprovision ratio = 6.360%
> > Info: Overprovision segments = 68 (GC reserved = 39)
> > Info: format successful
> > 
> > $hexdump -s 0x10000 -n 128 -C test.img
> > 00010000  c2 8a c8 26 00 00 00 00  00 00 00 00 00 00 00 00  |...&............|
> > 00010010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > 00010020  92 ab 3f c6 b7 82 49 5e  93 23 e8 c9 e9 45 7d ac  |..?...I^.#...E}.|
> > 00010030  00 00 01 00 00 00 00 00  01 00 00 00 00 00 00 00  |................|
> > 00010040  5f 42 48 52 66 53 5f 4d  05 00 00 00 00 00 00 00  |_BHRfS_M........|
> > 00010050  00 40 d2 01 00 00 00 00  00 40 50 01 00 00 00 00  |.@.......@P.....|
> > 00010060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > 00010070  00 00 00 40 00 00 00 00  00 00 02 00 00 00 00 00  |...@............|
> > 00010080
> > 
> > --- After this patch ---
> > $ mkfs.f2fs -t 0 -f test.img
> > 
> > 	F2FS-tools: mkfs.f2fs Ver: 1.14.0 (2020-12-28)
> > 
> > Info: Disable heap-based policy
> > Info: Debug level = 0
> > Info: Trim is disabled
> > 	test.img appears to contain an existing filesystem (btrfs).
> > Info: Segments per section = 1
> > Info: Sections per zone = 1
> > Info: sector size = 512
> > Info: total sectors = 2097152 (1024 MB)
> > Info: zone aligned segment0 blkaddr: 512
> > Info: format version with
> >   "Linux version 5.10.46-4rodete1-amd64 (glinux-team@google.com) (gcc-10 (Debian 10.2.1-6+build2) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2) #1 SMP Debian 5.10.46-4rodete1 (2021-08-20)"
> > Info: Overprovision ratio = 6.360%
> > Info: Overprovision segments = 68 (GC reserved = 39)
> > Info: format successful
> > 
> > $ hexdump -s 0x10000 -n 128 -C test.img
> > 00010000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00010080
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fsck/main.c             |  2 +-
> >  include/f2fs_fs.h       |  1 +
> >  lib/libf2fs.c           |  6 ++++++
> >  mkfs/f2fs_format_main.c | 26 ++++++++++++++++++++++----
> >  4 files changed, 30 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fsck/main.c b/fsck/main.c
> > index eda399cf0679..e4cfdf443867 100644
> > --- a/fsck/main.c
> > +++ b/fsck/main.c
> > @@ -1120,7 +1120,7 @@ int main(int argc, char **argv)
> >  	}
> >  
> >  	/* Get device */
> > -	if (f2fs_get_device_info() < 0) {
> > +	if (f2fs_get_device_info() < 0 || f2fs_get_f2fs_info() < 0) {
> >  		ret = -1;
> >  		if (c.func == FSCK)
> >  			ret = FSCK_OPERATIONAL_ERROR;
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index 2a9c1169456c..69260a6fccd9 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -1341,6 +1341,7 @@ extern int f2fs_devs_are_umounted(void);
> >  extern int f2fs_dev_is_writable(void);
> >  extern int f2fs_dev_is_umounted(char *);
> >  extern int f2fs_get_device_info(void);
> > +extern int f2fs_get_f2fs_info(void);
> >  extern unsigned int calc_extra_isize(void);
> >  extern int get_device_info(int);
> >  extern int f2fs_init_sparse_file(void);
> > diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> > index b27785dec757..420dfdae56da 100644
> > --- a/lib/libf2fs.c
> > +++ b/lib/libf2fs.c
> > @@ -1183,6 +1183,12 @@ int f2fs_get_device_info(void)
> >  	for (i = 0; i < c.ndevs; i++)
> >  		if (get_device_info(i))
> >  			return -1;
> > +	return 0;
> > +}
> > +
> > +int f2fs_get_f2fs_info(void)
> > +{
> > +	int i;
> >  
> >  	if (c.wanted_total_sectors < c.total_sectors) {
> >  		MSG(0, "Info: total device sectors = %"PRIu64" (in %u bytes)\n",
> > diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
> > index 031244d5d67a..de93f1b5b778 100644
> > --- a/mkfs/f2fs_format_main.c
> > +++ b/mkfs/f2fs_format_main.c
> > @@ -396,18 +396,36 @@ int main(int argc, char *argv[])
> >  
> >  	c.func = MKFS;
> >  
> > -	if (!force_overwrite && f2fs_check_overwrite()) {
> > -		MSG(0, "\tUse the -f option to force overwrite.\n");
> > +	if (f2fs_get_device_info() < 0)
> >  		return -1;
> 
> Jaegeuk, thanks for the patch. Today, I had a chance to try out the dev branch
> with this patch, and observed mkfs.f2fs for block devices failed with the
> messages as follows:
> 
> $ sudo mkfs.f2fs -f /dev/sdc
> 
>         F2FS-tools: mkfs.f2fs Ver: 1.14.0 (2021-09-10)
> 
> Info: Disable heap-based policy
> Info: Debug level = 0
> Info: Trim is enabled
> Info: [/dev/sdc] Disk Model: SanDisk SDSSDH32
>         /dev/sdc appears to contain an existing filesystem (f2fs).
>         Error: In use by the system!
> 
> I think f2fs_get_device_info() call above opens the block device with O_EXCL.
> And the following f2fs_devs_are_umounted() opens the device again with O_EXCL.
> The second open results in E_BUSY and the error message. I tried to move
> the f2fs_devs_are_umounted() call before the f2fs_get_device_info() call, then
> the error disappeared. So, I suggest to move the f2fs_devs_are_umounted() call.

Thank you. Could you please take a look at v2?

> 
> > +
> > +	if (f2fs_check_overwrite()) {
> > +		char *zero_buf = NULL;
> > +		int i;
> > +
> > +		if (!force_overwrite) {
> > +			MSG(0, "\tUse the -f option to force overwrite.\n");
> > +			goto err_format;
> > +		}
> > +		zero_buf = calloc(F2FS_BLKSIZE, 1);
> > +		if (!zero_buf) {
> > +			MSG(0, "\tFaile to allocate zero buffer.\n");
> 
> I guess you meant "Failed" in place of "Faile".
> 
> > +			goto err_format;
> > +		}
> > +		/* wipe out other FS magics mostly first 4MB space */
> > +		for (i = 0; i < 1024; i++)
> > +			dev_fill_block(zero_buf, i);
> > +		free(zero_buf);
> > +		f2fs_fsync_device();
> >  	}
> >  
> >  	if (f2fs_devs_are_umounted() < 0) {
> >  		if (errno != EBUSY)
> >  			MSG(0, "\tError: Not available on mounted device!\n");
> > -		return -1;
> > +		goto err_format;
> >  	}
> >  
> > -	if (f2fs_get_device_info() < 0)
> > +	if (f2fs_get_f2fs_info() < 0)
> >  		goto err_format;
> >  
> >  	/*
> > -- 
> > 2.33.0.309.g3052b89438-goog
> > 
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> -- 
> Best Regards,
> Shin'ichiro Kawasaki


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2021-09-20 23:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 22:56 [f2fs-dev] [PATCH] mkfs.f2fs: wipe other FS magics given -f Jaegeuk Kim
2021-09-15  5:45 ` Shinichiro Kawasaki
2021-09-20 23:26   ` Jaegeuk Kim [this message]
2021-09-21  1:31     ` Shinichiro Kawasaki
2021-09-16 10:59 ` Chao Yu
2021-09-20 23:26 ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
2021-09-22 15:25   ` Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YUkYwQidnXGB9LLk@google.com \
    --to=jaegeuk@kernel.org \
    --cc=Damien.LeMoal@wdc.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=shinichiro.kawasaki@wdc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.