| Message ID | 54170F35.4040103@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Anand On 15/09/14 17:09, Anand Jain wrote: > > Sam, > > Thanks for reporting. Can you apply the following diff > on top of 3.17rc5 and check if it helps. The patch fixes the issue, although it took a little tweaking to get it to apply cleanly. > ------- > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index e9676a4..1224b61 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -533,7 +533,7 @@ static noinline int device_list_add(const char *path, > * the btrfs dev scan cli, after FS has been mounted. > */ > if (fs_devices->opened) { > - return -EBUSY; > + goto out; > } else { > /* > * That is if the FS is _not_ mounted and if you > @@ -566,6 +566,7 @@ static noinline int device_list_add(const char *path, > if (!fs_devices->opened) > device->generation = found_transid; > > +out: > *fs_devices_ret = fs_devices; > > return ret; > ------- > > > > > Anand > > > > On 15/09/2014 23:54, Chris Mason wrote: >> On 09/15/2014 11:13 AM, Sam Thursfield wro: >>> Hi! >>> >>> I'm having an issue with the 3.17rc5 kernel which prevents having >>> multiple subvolumes of the same disk mounted. >>> >>> I'm not sure exactly the cause. I thought it might be because in my >>> system the root file system is itself a subvolume of the disk I'm trying >>> to mount. But if I create a second disk image with two subvolumes, the >>> same thing occurs -- I can only mount one of them at a time. >>> >>> I've attached the output of a few commands, please let me know if you >>> want more info (I'm not subscribed to the list, please keep me in To:). >>> >>> We've bisected and found that the exact commit that changed the >>> behaviour is this one: >>> https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id%3Db96de000bc8bc9688b3a2abea4332bd57648a49f&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=6%2FL0lzzDhu0Y1hL9xm%2BQyA%3D%3D%0A&m=qVtp3yxxUd8uElAkp118K4Bd0oZfeUOC%2BKUy3e6rRlA%3D%0A&s=0d2623956de100adc1185d2b75e9114384572b88c0330e74924baa1a2bce8d02 >>> >>> >>> >>> I'm afraid I'm not familiar with the Btrfs code base and so I don't >>> understand the exact meaning of the comments in that commit. >> >> Anand Jain is working on this in a separate thread. I'll make sure the >> fix goes into the next rc, thanks for all the time spent bisecting. >> >> -chris >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >>
Hi Sam, Thanks for verifying. On 16/09/2014 01:13, Sam Thursfield wrote: > Hi Anand > > On 15/09/14 17:09, Anand Jain wrote: >> >> Sam, >> >> Thanks for reporting. Can you apply the following diff >> on top of 3.17rc5 and check if it helps. > > The patch fixes the issue, although it took a little tweaking to get it > to apply cleanly. > >> ------- >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index e9676a4..1224b61 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -533,7 +533,7 @@ static noinline int device_list_add(const char *path, >> * the btrfs dev scan cli, after FS has been mounted. >> */ >> if (fs_devices->opened) { >> - return -EBUSY; >> + goto out; >> } else { >> /* >> * That is if the FS is _not_ mounted and if you >> @@ -566,6 +566,7 @@ static noinline int device_list_add(const char *path, >> if (!fs_devices->opened) >> device->generation = found_transid; >> >> +out: >> *fs_devices_ret = fs_devices; >> >> return ret; >> ------- >> >> >> >> >> Anand >> >> >> >> On 15/09/2014 23:54, Chris Mason wrote: >>> On 09/15/2014 11:13 AM, Sam Thursfield wro: >>>> Hi! >>>> >>>> I'm having an issue with the 3.17rc5 kernel which prevents having >>>> multiple subvolumes of the same disk mounted. >>>> >>>> I'm not sure exactly the cause. I thought it might be because in my >>>> system the root file system is itself a subvolume of the disk I'm >>>> trying >>>> to mount. But if I create a second disk image with two subvolumes, the >>>> same thing occurs -- I can only mount one of them at a time. >>>> >>>> I've attached the output of a few commands, please let me know if you >>>> want more info (I'm not subscribed to the list, please keep me in To:). >>>> >>>> We've bisected and found that the exact commit that changed the >>>> behaviour is this one: >>>> https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id%3Db96de000bc8bc9688b3a2abea4332bd57648a49f&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=6%2FL0lzzDhu0Y1hL9xm%2BQyA%3D%3D%0A&m=qVtp3yxxUd8uElAkp118K4Bd0oZfeUOC%2BKUy3e6rRlA%3D%0A&s=0d2623956de100adc1185d2b75e9114384572b88c0330e74924baa1a2bce8d02 >>>> >>>> >>>> >>>> >>>> I'm afraid I'm not familiar with the Btrfs code base and so I don't >>>> understand the exact meaning of the comments in that commit. >>> >>> Anand Jain is working on this in a separate thread. I'll make sure the >>> fix goes into the next rc, thanks for all the time spent bisecting. >>> >>> -chris >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> linux-btrfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> The patch fixes the issue, although it took a little tweaking to get it >> to apply cleanly. Sam, In the above context, Can you pls share the btrfs fi show output ? Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This diff on top of 3.17rc5 also fixes the issue on my box (encrypted /home subvolume) Xavier > > Hi Sam, > > Thanks for verifying. > > On 16/09/2014 01:13, Sam Thursfield wrote: >> Hi Anand >> >> On 15/09/14 17:09, Anand Jain wrote: >>> >>> Sam, >>> >>> Thanks for reporting. Can you apply the following diff >>> on top of 3.17rc5 and check if it helps. >> >> The patch fixes the issue, although it took a little tweaking to get it >> to apply cleanly. >> >>> ------- >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index e9676a4..1224b61 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -533,7 +533,7 @@ static noinline int device_list_add(const char >>> *path, >>> * the btrfs dev scan cli, after FS has been mounted. >>> */ >>> if (fs_devices->opened) { >>> - return -EBUSY; >>> + goto out; >>> } else { >>> /* >>> * That is if the FS is _not_ mounted and >>> if you >>> @@ -566,6 +566,7 @@ static noinline int device_list_add(const char >>> *path, >>> if (!fs_devices->opened) >>> device->generation = found_transid; >>> >>> +out: >>> *fs_devices_ret = fs_devices; >>> >>> return ret; >>> ------- >>> >>> >>> >>> >>> Anand >>> >>> >>> >>> On 15/09/2014 23:54, Chris Mason wrote: >>>> On 09/15/2014 11:13 AM, Sam Thursfield wro: >>>>> Hi! >>>>> >>>>> I'm having an issue with the 3.17rc5 kernel which prevents having >>>>> multiple subvolumes of the same disk mounted. >>>>> >>>>> I'm not sure exactly the cause. I thought it might be because in my >>>>> system the root file system is itself a subvolume of the disk I'm >>>>> trying >>>>> to mount. But if I create a second disk image with two subvolumes, >>>>> the >>>>> same thing occurs -- I can only mount one of them at a time. >>>>> >>>>> I've attached the output of a few commands, please let me know if you >>>>> want more info (I'm not subscribed to the list, please keep me in >>>>> To:). >>>>> >>>>> We've bisected and found that the exact commit that changed the >>>>> behaviour is this one: >>>>> https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id%3Db96de000bc8bc9688b3a2abea4332bd57648a49f&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=6%2FL0lzzDhu0Y1hL9xm%2BQyA%3D%3D%0A&m=qVtp3yxxUd8uElAkp118K4Bd0oZfeUOC%2BKUy3e6rRlA%3D%0A&s=0d2623956de100adc1185d2b75e9114384572b88c0330e74924baa1a2bce8d02 >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> I'm afraid I'm not familiar with the Btrfs code base and so I don't >>>>> understand the exact meaning of the comments in that commit. >>>> >>>> Anand Jain is working on this in a separate thread. I'll make sure >>>> the >>>> fix goes into the next rc, thanks for all the time spent bisecting. >>>> >>>> -chris >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe >>>> linux-btrfs" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2014 12:09 PM, Anand Jain wrote: > > Sam, > > Thanks for reporting. Can you apply the following diff > on top of 3.17rc5 and check if it helps. > > ------- > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index e9676a4..1224b61 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -533,7 +533,7 @@ static noinline int device_list_add(const char *path, > * the btrfs dev scan cli, after FS has been mounted. > */ > if (fs_devices->opened) { > - return -EBUSY; > + goto out; > } else { > /* > * That is if the FS is _not_ mounted and if you > @@ -566,6 +566,7 @@ static noinline int device_list_add(const char *path, > if (!fs_devices->opened) > device->generation = found_transid; > > +out: > *fs_devices_ret = fs_devices; > > return ret; Anand, are you planning on sending a full patch out for this? One concern I have is that after the device_list_add call: if (!ret && fs_devices_ret) (*fs_devices_ret)->total_devices = total_devices; We should only be doing this from the newest super, not blindly overwriting. But that's a merge window fix. For now I just want to deal with the regression, and your patch above looks good. Thanks for jumping on this one. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 15.09.2014 um 19:42 schrieb Anand Jain: > Sam, > > In the above context, Can you pls share the btrfs fi show output ? unsure if it helps: I applied the patch as well, running gentoo on latest git-sources here. My "btrfs fi show" misses my SSD now: # btrfs fi show Label: 'btrfs_evo' uuid: ea95dbd1-ef4e-48a4-9732-54e6c80b31df Total devices 1 FS bytes used 80.91GiB *** Some devices missing .. although all the btrfs-subvols on /dev/sda2 are mounted correctly and everything works fine so far. Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stefan,
Can you pls send me the output for
cat /etc/fstab | egrep btrfs
cat /proc/self/mounts | egrep btrfs
mount | egrep btrfs
Thanks, Anand
On 17/09/2014 05:08, Stefan G. Weichinger wrote:
> Am 15.09.2014 um 19:42 schrieb Anand Jain:
>
>> Sam,
>>
>> In the above context, Can you pls share the btrfs fi show output ?
>
>
> unsure if it helps:
>
> I applied the patch as well, running gentoo on latest git-sources here.
>
> My "btrfs fi show" misses my SSD now:
>
> # btrfs fi show
> Label: 'btrfs_evo' uuid: ea95dbd1-ef4e-48a4-9732-54e6c80b31df
> Total devices 1 FS bytes used 80.91GiB
> *** Some devices missing
>
> .. although all the btrfs-subvols on /dev/sda2 are mounted correctly and
> everything works fine so far.
>
> Stefan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks Sam for the logs. In your config btrfs Kernel still knows the device as /dev/root. not /dev/sda. looks like during boot up the device is /dev/root and after boot up its /dev/sda. so is the problem all about. some logs below to support that. Unfortunately at my end I am not able to reproduce this on a virtual box. I guess it would need a real bios. If you have any of that info that will help. when progs can't find the device in user space it would print missing. That was introduced in patch 206efb60cbe3049e0d44c6da3c1909aeee18f813. But the point to note is - though the actual device is same, its path during boot up and after boot up are different in some standalone systems. scsi inquiry serial number provides an unique device id. But in btrfs we are depend on our SB. which can copied across devices. OR there might some un-wiped stale SB on the device. We need some new mechanism to handle this. From your system, /proc/self/mounts (the device path here comes from the btrfs kernel, ) ------ /dev/root / btrfs ... :: /dev/root /home btrfs rw,noatime,compress=lzo,ssd,space_cache 0 0 ------- per mount cli ----- /dev/sda2 on / type btrfs ... :: /dev/sda2 on /home type btrfs ... ------ fstab ----- UUID=..b31df / btrfs defaults,noatime,compress=lzo,subvolid=258 :: UUID=..b31df /home btrfs defaults,noatime,compress=lzo,subvolid=285 ------- Anand On 17/09/2014 05:53, Anand Jain wrote: > > > Stefan, > > Can you pls send me the output for > > cat /etc/fstab | egrep btrfs > cat /proc/self/mounts | egrep btrfs > mount | egrep btrfs > > Thanks, Anand > > > > On 17/09/2014 05:08, Stefan G. Weichinger wrote: >> Am 15.09.2014 um 19:42 schrieb Anand Jain: >> >>> Sam, >>> >>> In the above context, Can you pls share the btrfs fi show output ? >> >> >> unsure if it helps: >> >> I applied the patch as well, running gentoo on latest git-sources here. >> >> My "btrfs fi show" misses my SSD now: >> >> # btrfs fi show >> Label: 'btrfs_evo' uuid: ea95dbd1-ef4e-48a4-9732-54e6c80b31df >> Total devices 1 FS bytes used 80.91GiB >> *** Some devices missing >> >> .. although all the btrfs-subvols on /dev/sda2 are mounted correctly and >> everything works fine so far. >> >> Stefan >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Chris, >> ------- >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index e9676a4..1224b61 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -533,7 +533,7 @@ static noinline int device_list_add(const char *path, >> * the btrfs dev scan cli, after FS has been mounted. >> */ >> if (fs_devices->opened) { >> - return -EBUSY; >> + goto out; >> } else { >> /* >> * That is if the FS is _not_ mounted and if you >> @@ -566,6 +566,7 @@ static noinline int device_list_add(const char *path, >> if (!fs_devices->opened) >> device->generation = found_transid; >> >> +out: >> *fs_devices_ret = fs_devices; >> >> return ret; > > Anand, are you planning on sending a full patch out for this? One concern > I have is that after the device_list_add call: > > > if (!ret && fs_devices_ret) > (*fs_devices_ret)->total_devices = total_devices; > > We should only be doing this from the newest super, not blindly overwriting. > But that's a merge window fix. For now I just want to deal with the regression, > and your patch above looks good. > > Thanks for jumping on this one. Sorry for the trouble. yes, I will be sending a full patch. I am finding too difficult to revive the function btrfs_scan_one_device() which is predominately to handle device scan and list_update _before_ any mount. Further to the concern which you mention above, there is Ioctl BTRFS_IOC_DEV_READY also using this function, which absolutely should not have any intention to update the device list, but it does .. theoretically. And I note that this ioctl is used by systemd as well. So the fix is getting a bit complicated. I am attempting. Thanks, Anand > -chris > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/17/2014 05:47 AM, Anand Jain wrote: > > Hi Chris, > > >>> ------- >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index e9676a4..1224b61 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -533,7 +533,7 @@ static noinline int device_list_add(const char >>> *path, >>> * the btrfs dev scan cli, after FS has been mounted. >>> */ >>> if (fs_devices->opened) { >>> - return -EBUSY; >>> + goto out; >>> } else { >>> /* >>> * That is if the FS is _not_ mounted and if >>> you >>> @@ -566,6 +566,7 @@ static noinline int device_list_add(const char >>> *path, >>> if (!fs_devices->opened) >>> device->generation = found_transid; >>> >>> +out: >>> *fs_devices_ret = fs_devices; >>> >>> return ret; >> >> Anand, are you planning on sending a full patch out for this? One >> concern >> I have is that after the device_list_add call: >> >> >> if (!ret && fs_devices_ret) >> (*fs_devices_ret)->total_devices = total_devices; >> >> We should only be doing this from the newest super, not blindly >> overwriting. >> But that's a merge window fix. For now I just want to deal with the >> regression, >> and your patch above looks good. >> >> Thanks for jumping on this one. > > > Sorry for the trouble. > yes, I will be sending a full patch. I am finding too difficult > to revive the function btrfs_scan_one_device() which is predominately > to handle device scan and list_update _before_ any mount. Further > to the concern which you mention above, there is Ioctl > BTRFS_IOC_DEV_READY also using this function, which absolutely should > not have any intention to update the device list, but it does .. > theoretically. And I note that this ioctl is used by systemd as well. > So the fix is getting a bit complicated. I am attempting. No problem, the original patch looked right to me too. We're getting closer to rc6, I think at this point I'll revert the original patch until the next merge window. Then we can step back and nail down exactly what is going on. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 17.09.2014 um 15:21 schrieb Chris Mason: > No problem, the original patch looked right to me too. We're getting > closer to rc6, I think at this point I'll revert the original patch > until the next merge window. Then we can step back and nail down > exactly what is going on. running git-sources 3.17-rc6 now and didn't need to apply the patch. So you fixed it already? "btrfs fi show" also displays devices correctly again. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index e9676a4..1224b61 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -533,7 +533,7 @@ static noinline int device_list_add(const char *path, * the btrfs dev scan cli, after FS has been mounted. */ if (fs_devices->opened) { - return -EBUSY; + goto out; } else { /* * That is if the FS is _not_ mounted and if you @@ -566,6 +566,7 @@ static noinline int device_list_add(const char *path, if (!fs_devices->opened) device->generation = found_transid; +out: *fs_devices_ret = fs_devices; return ret;