| Message ID | 20191113102728.8835-5-jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/7] btrfs: decrement number of open devices after closing the device not before | expand |
On Wed, Nov 13, 2019 at 11:27:25AM +0100, Johannes Thumshirn wrote: > close_fs_devices() will be able to return an error instead of crashing > after the following patch. > > Prepare btrfs_close_devices() for this. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > fs/btrfs/volumes.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index e5864ca3bb3b..be1fd935edf7 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1143,10 +1143,10 @@ static int close_fs_devices(struct btrfs_fs_devices *fs_devices) > int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > { > struct btrfs_fs_devices *seed_devices = NULL; > - int ret; > + int err, err2 = 0; Please use ret and ret2. > mutex_lock(&uuid_mutex); > - ret = close_fs_devices(fs_devices); > + err = close_fs_devices(fs_devices); > if (!fs_devices->opened) { > seed_devices = fs_devices->seed; > fs_devices->seed = NULL; > @@ -1156,10 +1156,13 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > while (seed_devices) { > fs_devices = seed_devices; > seed_devices = fs_devices->seed; > - close_fs_devices(fs_devices); > + err2 = close_fs_devices(fs_devices); So only the last error value gets propagated to the return statements. Is that intentional? > free_fs_devices(fs_devices); > } > - return ret; > + > + if (err2) > + return err2; > + return err;
On 13/11/2019 16:00, David Sterba wrote: > On Wed, Nov 13, 2019 at 11:27:25AM +0100, Johannes Thumshirn wrote: >> close_fs_devices() will be able to return an error instead of crashing >> after the following patch. >> >> Prepare btrfs_close_devices() for this. >> >> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> >> --- >> fs/btrfs/volumes.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index e5864ca3bb3b..be1fd935edf7 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -1143,10 +1143,10 @@ static int close_fs_devices(struct btrfs_fs_devices *fs_devices) >> int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) >> { >> struct btrfs_fs_devices *seed_devices = NULL; >> - int ret; >> + int err, err2 = 0; > > Please use ret and ret2. Sure. >> mutex_lock(&uuid_mutex); >> - ret = close_fs_devices(fs_devices); >> + err = close_fs_devices(fs_devices); >> if (!fs_devices->opened) { >> seed_devices = fs_devices->seed; >> fs_devices->seed = NULL; >> @@ -1156,10 +1156,13 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) >> while (seed_devices) { >> fs_devices = seed_devices; >> seed_devices = fs_devices->seed; >> - close_fs_devices(fs_devices); >> + err2 = close_fs_devices(fs_devices); > > So only the last error value gets propagated to the return statements. > Is that intentional? Yes it was. Even if the first close_fs_devices() call fails, we can still try to close eventual seed devices. If this fails as well, we return the second error. If it succeeds we'll return the first error (or 0). Byte, Johannes
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index e5864ca3bb3b..be1fd935edf7 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1143,10 +1143,10 @@ static int close_fs_devices(struct btrfs_fs_devices *fs_devices) int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) { struct btrfs_fs_devices *seed_devices = NULL; - int ret; + int err, err2 = 0; mutex_lock(&uuid_mutex); - ret = close_fs_devices(fs_devices); + err = close_fs_devices(fs_devices); if (!fs_devices->opened) { seed_devices = fs_devices->seed; fs_devices->seed = NULL; @@ -1156,10 +1156,13 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) while (seed_devices) { fs_devices = seed_devices; seed_devices = fs_devices->seed; - close_fs_devices(fs_devices); + err2 = close_fs_devices(fs_devices); free_fs_devices(fs_devices); } - return ret; + + if (err2) + return err2; + return err; } static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
close_fs_devices() will be able to return an error instead of crashing after the following patch. Prepare btrfs_close_devices() for this. Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- fs/btrfs/volumes.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)