Re: remount and conflicting ssd options?

2017-04-15 Thread Hans van Kranenburg
On 04/15/2017 10:35 PM, Chris Murphy wrote:
> On Sat, Apr 15, 2017 at 12:50 PM, Adam Borowski  wrote:
>> On Sat, Apr 15, 2017 at 12:41:14PM -0600, Chris Murphy wrote:
>>> On Sat, Apr 15, 2017 at 12:31 PM, Adam Borowski  wrote:
 On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
> Then later I tried
>
> mount -o remount,ssd
>
> To go back to regular ssd option, but nothing happens, mount still
> shows ssd_spread.

 ssd_spread implies ssd.
>>>
>>> OK so it's possible to remount from ssd to ssd_spread but not back to
>>> ssd? If I trust the mount output, it's only possible to transition
>>> from ssd to ssd_spread, but not from ssd_spread to ssd.
>>
>> Yeah.  This is a very minor problem (workaround: remount with nossd then
>> ssd), but in the light of Hans van Kranenburg's findings, it'd be a bad idea
>> to introduce new mount options the kernel would need to keep recognizing
>> forever, as the effect of those options needs some rethinking anyways.
> 
> Yeah it's confusing aside from the mount behavior.
> 
> I have a performance test based on doing a bunch of system updates
> (~100 rpm packages), and even with the same mount options, back to
> back tests are sufficiently inconsistent that I can't tell which of
> the allocator options is better.

Do you run the test with exact the same starting point (all bits on disk
in the same place, drop caches), or do you run it over and over again
while the filesystem is deepening its own grave every time?

> So the allocation options must be
> about something other than time based performance.

They're about the size of free space fragments that are (re)used.
Switching allocator policy on the same fs rapidly might give weird
results because some fragmented free space that was left behind by the
previous test might suddenly show up as delicious supper for the next.

> I can't tell what is a slow ssd these days so I'm not sure what ssd vs
> ssd_spread really mean.

afaikrn:
ssd = ignore 'small' free space fragments ('small' being something I
haven't figured out yet)
ssd_spread = do not split up a write in multiple extents

> Plus then on spinning rust people have
> reported better performance with ssd rather than the default of nossd.
> While others who are using iSCSI end up with ssd by default and worse
> results than with nossd.

So far we learned that the options were designed for the types
of SSD that you would be able to buy 10 years ago, that they cause
changes in write behaviour and that they tend to forget to take
(recursive-exploding) cow overhead for metadata writes into account.

-- 
Hans van Kranenburg
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Encountered kernel bug#72811. Advice on recovery?

2017-04-15 Thread Hugo Mills
On Sat, Apr 15, 2017 at 11:28:41PM +, Duncan wrote:
> Duncan posted on Sat, 15 Apr 2017 01:41:28 + as excerpted:
> 
> > Besides which, if the patch was submitted now, the earliest it could
> > really hit btrfs-progs would be 4.12,
> 
> Well, maybe 3.11.x...

   Can I borrow your time machine? Would last Wednesday be OK?

   Hugo.

-- 
Hugo Mills | We teach people management skills by examining
hugo@... carfax.org.uk | characters in Shakespeare. You could look at
http://carfax.org.uk/  | Claudius's crisis management techniques, for
PGP: E2AB1DE4  | example.   Richard Smith-Jones, Slings and Arrows


signature.asc
Description: Digital signature


Re: Encountered kernel bug#72811. Advice on recovery?

2017-04-15 Thread Duncan
Duncan posted on Sat, 15 Apr 2017 01:41:28 + as excerpted:

> Besides which, if the patch was submitted now, the earliest it could
> really hit btrfs-progs would be 4.12,

Well, maybe 3.11.x...



-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Chris Murphy
On Sat, Apr 15, 2017 at 12:50 PM, Adam Borowski  wrote:
> On Sat, Apr 15, 2017 at 12:41:14PM -0600, Chris Murphy wrote:
>> On Sat, Apr 15, 2017 at 12:31 PM, Adam Borowski  wrote:
>> > On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
>> >> Then later I tried
>> >>
>> >> mount -o remount,ssd
>> >>
>> >> To go back to regular ssd option, but nothing happens, mount still
>> >> shows ssd_spread.
>> >
>> > ssd_spread implies ssd.
>>
>> OK so it's possible to remount from ssd to ssd_spread but not back to
>> ssd? If I trust the mount output, it's only possible to transition
>> from ssd to ssd_spread, but not from ssd_spread to ssd.
>
> Yeah.  This is a very minor problem (workaround: remount with nossd then
> ssd), but in the light of Hans van Kranenburg's findings, it'd be a bad idea
> to introduce new mount options the kernel would need to keep recognizing
> forever, as the effect of those options needs some rethinking anyways.

Yeah it's confusing aside from the mount behavior.

I have a performance test based on doing a bunch of system updates
(~100 rpm packages), and even with the same mount options, back to
back tests are sufficiently inconsistent that I can't tell which of
the allocator options is better. So the allocation options must be
about something other than time based performance.

I can't tell what is a slow ssd these days so I'm not sure what ssd vs
ssd_spread really mean. Plus then on spinning rust people have
reported better performance with ssd rather than the default of nossd.
While others who are using iSCSI end up with ssd by default and worse
results than with nossd.



-- 
Chris Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] btrfs: drop the nossd flag when remounting with -o ssd

2017-04-15 Thread Hans van Kranenburg
On 03/31/2017 07:10 PM, David Sterba wrote:
> On Fri, Mar 31, 2017 at 06:00:08PM +0200, Hans van Kranenburg wrote:
>> On 03/31/2017 05:19 PM, Adam Borowski wrote:
>>> The opposite case was already handled right in the very next switch entry.
>>>
>>> Reported-by: Hans van Kranenburg 
>>> Signed-off-by: Adam Borowski 
>>> ---
>>> Not sure if setting NOSSD should also disable SSD_SPREAD, there's currently
>>> no way to disable that option once set.
> 
> Missing inverse of ssd_spread is probably unintentional, as we once
> added all complementary no* options, this one was forgotten.
> 
> And yes, nossd should turn off ssd and ssd_spread, as ssd_spread without
> ssd does not nothing anyway.

ssd_spread is tested exactly one time, in free-space-cache.c in
btrfs_find_space_cluster

As far as I can quickly find out, that code is reached regardless of
which other options are set.

So, it *does* something anyway if ssd is not set. And I think it
prevents all writes from being split into multiple extents.

So, nossd,ssd_spread (no ssd) is right now something that you can end up
with (when the patch here is not applied) when playing with -o remount.

> 
>>>
>>>  fs/btrfs/super.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>> index 06bd9b332e18..7342399951ad 100644
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -549,11 +549,13 @@ int btrfs_parse_options(struct btrfs_fs_info *info, 
>>> char *options,
>>> case Opt_ssd:
>>> btrfs_set_and_info(info, SSD,
>>>"use ssd allocation scheme");
>>> +   btrfs_clear_opt(info->mount_opt, NOSSD);
>>> break;
>>> case Opt_ssd_spread:
>>> btrfs_set_and_info(info, SSD_SPREAD,
>>>"use spread ssd allocation scheme");
>>> btrfs_set_opt(info->mount_opt, SSD);
>>> +   btrfs_clear_opt(info->mount_opt, NOSSD);
>>> break;
>>> case Opt_nossd:
>>> btrfs_set_and_info(info, NOSSD,
>>
>> How did you test this?
>>
>> This was also my first thought, but here's a weird thing:
>>
>> -# mount -o nossd /dev/sdx /mnt/btrfs/
>>
>> BTRFS info (device sdx): not using ssd allocation scheme
>>
>> -# mount -o remount,ssd /mnt/btrfs/
>>
>> BTRFS info (device sdx): use ssd allocation scheme
>>
>> -# mount -o remount,nossd /mnt/btrfs/
>>
>> BTRFS info (device sdx): use ssd allocation scheme
>>
>> That means that the case Opt_nossd: is never reached when doing this?
>>
>> And... what should be the result of doing:
>> -# mount -o remount,nossd,ssd /mnt/btrfs/
>>
>> I guess it should be that the last one in the sequence wins?
> 
> The last one wins.
> 
>> The fact that nossd,ssd,ssd_spread are different options complicates the
>> whole thing, compared to e.g. autodefrag, noautodefrag.
> 
> I think the the ssd flags reflect the autodetection of ssd, unlike
> autodefrag and others.
> 
> The ssd options says "enable the ssd mode", but it could be also
> auto-detected if the non-rotational device is detected.
> 
> nossd says, "do not do the autodetection, even if it's a non-rot
> device, also disable all ssd modes".
> 
> The manual page is not entirely clear about that, I'll update it.
> 
> So Adam's patch needs to be updated so NOSSD also disables SSD_SPREAD.
> Adding the 'nossd_spread' would be good to have, even if it might be
> just a marginal usecase.
> 


-- 
Hans van Kranenburg
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Hans van Kranenburg
On 04/15/2017 08:41 PM, Chris Murphy wrote:
> On Sat, Apr 15, 2017 at 12:31 PM, Adam Borowski  wrote:
>> On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
>>> I don't understand this:
>>>
>>> /dev/mmcblk0p3 on / type btrfs
>>> (rw,noatime,seclabel,compress=zlib,nossd,ssd_spread,space_cache,commit=150,subvolid=260,subvol=/root)
>>>
>>> The fstab uses ssd_spread. It looks like during startup the initial
>>> option is ssd via autodetection and then at switchroot time, when it
>>> goes ro to rw, the fstab options are applied and it becomes
>>> ssd_spread. Fine.
>>>
>>> Then later I tried
>>>
>>> mount -o remount,ssd
>>>
>>> To go back to regular ssd option, but nothing happens, mount still
>>> shows ssd_spread.
>>
>> ssd_spread implies ssd.

Well, not really. ssd_spread also triggers ssd to be set to enabled, but
they're two different flags, and they're being tested separately in the
code. (!)

> 
> OK so it's possible to remount from ssd to ssd_spread but not back to
> ssd? If I trust the mount output, it's only possible to transition
> from ssd to ssd_spread, but not from ssd_spread to ssd.

That's correct. That's a bug. I think it needs a full umount to get it away.

>>
>>> And then if I do
>>>
>>> mount -o remount,nossd
>>>
>>> I get the above mount output with nossd,ssd_spread options which would
>>> seem to be a contradiction. At least it's confusing. So... now what?

As far as I understand now, this will mean that both nossd and
ssd_spread is active, which means that writes need a size + 0 (data) or
size + 64kiB (metadata) amount of free space to go in, and at the same
time it does not allow the write to be split up in multiple extents? :o

>>>
>>> kernel 4.10.8-200.fc25.x86_64
>>
>> Already fixed in mainline.  You need linus/master from literally today if
>> your time zone is between +1 and +3 (inclusively), adjust by one day
>> accordingly if not.
>>
>> Or cherry-pick 951e79663.
> 
> OK thanks.
> 
> Is this just fixing a mount bug? Or also with kernel messaging? I see
> the enabling of ssd and spread, but I don't see a kernel message when
> unsetting them. For the kernel messaging to be consistent, it'd need
> to always report a message when allocation strategy is being changed.

+1

I still haven't tested the patch myself yet, but I would definitely
expect that you should see the "not using ssd allocation scheme" appearing.

-- 
Hans van Kranenburg
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Adam Borowski
On Sat, Apr 15, 2017 at 12:41:14PM -0600, Chris Murphy wrote:
> On Sat, Apr 15, 2017 at 12:31 PM, Adam Borowski  wrote:
> > On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
> >> Then later I tried
> >>
> >> mount -o remount,ssd
> >>
> >> To go back to regular ssd option, but nothing happens, mount still
> >> shows ssd_spread.
> >
> > ssd_spread implies ssd.
> 
> OK so it's possible to remount from ssd to ssd_spread but not back to
> ssd? If I trust the mount output, it's only possible to transition
> from ssd to ssd_spread, but not from ssd_spread to ssd.

Yeah.  This is a very minor problem (workaround: remount with nossd then
ssd), but in the light of Hans van Kranenburg's findings, it'd be a bad idea
to introduce new mount options the kernel would need to keep recognizing
forever, as the effect of those options needs some rethinking anyways.

> >
> >> And then if I do
> >>
> >> mount -o remount,nossd
> >>
> >> I get the above mount output with nossd,ssd_spread options which would
> >> seem to be a contradiction. At least it's confusing. So... now what?
> >>
> >> kernel 4.10.8-200.fc25.x86_64
> >
> > Already fixed in mainline.  You need linus/master from literally today if
> > your time zone is between +1 and +3 (inclusively), adjust by one day
> > accordingly if not.
> >
> > Or cherry-pick 951e79663.
> 
> OK thanks.
> 
> Is this just fixing a mount bug? Or also with kernel messaging?

The NOSSD option actually does nothing except for preventing SSD from being
enabled, so switching ssd<->nossd was only a messaging issue; there was no
way to go ssd_spread->ssd, though.

> I see the enabling of ssd and spread, but I don't see a kernel message
> when unsetting them.  For the kernel messaging to be consistent, it'd need
> to always report a message when allocation strategy is being changed.

It's working correctly now.  You are told what the new strategy is, with no
mention of what the old one was.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Chris Murphy
On Sat, Apr 15, 2017 at 12:31 PM, Adam Borowski  wrote:
> On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
>> I don't understand this:
>>
>> /dev/mmcblk0p3 on / type btrfs
>> (rw,noatime,seclabel,compress=zlib,nossd,ssd_spread,space_cache,commit=150,subvolid=260,subvol=/root)
>>
>> The fstab uses ssd_spread. It looks like during startup the initial
>> option is ssd via autodetection and then at switchroot time, when it
>> goes ro to rw, the fstab options are applied and it becomes
>> ssd_spread. Fine.
>>
>> Then later I tried
>>
>> mount -o remount,ssd
>>
>> To go back to regular ssd option, but nothing happens, mount still
>> shows ssd_spread.
>
> ssd_spread implies ssd.

OK so it's possible to remount from ssd to ssd_spread but not back to
ssd? If I trust the mount output, it's only possible to transition
from ssd to ssd_spread, but not from ssd_spread to ssd.


>
>> And then if I do
>>
>> mount -o remount,nossd
>>
>> I get the above mount output with nossd,ssd_spread options which would
>> seem to be a contradiction. At least it's confusing. So... now what?
>>
>> kernel 4.10.8-200.fc25.x86_64
>
> Already fixed in mainline.  You need linus/master from literally today if
> your time zone is between +1 and +3 (inclusively), adjust by one day
> accordingly if not.
>
> Or cherry-pick 951e79663.

OK thanks.

Is this just fixing a mount bug? Or also with kernel messaging? I see
the enabling of ssd and spread, but I don't see a kernel message when
unsetting them. For the kernel messaging to be consistent, it'd need
to always report a message when allocation strategy is being changed.

-- 
Chris Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Adam Borowski
On Sat, Apr 15, 2017 at 12:17:25PM -0600, Chris Murphy wrote:
> I don't understand this:
> 
> /dev/mmcblk0p3 on / type btrfs
> (rw,noatime,seclabel,compress=zlib,nossd,ssd_spread,space_cache,commit=150,subvolid=260,subvol=/root)
> 
> The fstab uses ssd_spread. It looks like during startup the initial
> option is ssd via autodetection and then at switchroot time, when it
> goes ro to rw, the fstab options are applied and it becomes
> ssd_spread. Fine.
> 
> Then later I tried
> 
> mount -o remount,ssd
> 
> To go back to regular ssd option, but nothing happens, mount still
> shows ssd_spread.

ssd_spread implies ssd.

> And then if I do
> 
> mount -o remount,nossd
> 
> I get the above mount output with nossd,ssd_spread options which would
> seem to be a contradiction. At least it's confusing. So... now what?
> 
> kernel 4.10.8-200.fc25.x86_64

Already fixed in mainline.  You need linus/master from literally today if
your time zone is between +1 and +3 (inclusively), adjust by one day
accordingly if not.

Or cherry-pick 951e79663.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: remount and conflicting ssd options?

2017-04-15 Thread Hans van Kranenburg
On 04/15/2017 08:17 PM, Chris Murphy wrote:
> I don't understand this:
> 
> /dev/mmcblk0p3 on / type btrfs
> (rw,noatime,seclabel,compress=zlib,nossd,ssd_spread,space_cache,commit=150,subvolid=260,subvol=/root)
> 
> 
> The fstab uses ssd_spread. It looks like during startup the initial
> option is ssd via autodetection and then at switchroot time, when it
> goes ro to rw, the fstab options are applied and it becomes
> ssd_spread. Fine.
> 
> Then later I tried
> 
> mount -o remount,ssd
> 
> To go back to regular ssd option, but nothing happens, mount still
> shows ssd_spread. And then if I do
> 
> mount -o remount,nossd
> 
> I get the above mount output with nossd,ssd_spread options which would
> seem to be a contradiction. At least it's confusing. So... now what?
> 
> kernel 4.10.8-200.fc25.x86_64
> 

See thread "Cosmetics bug: remounting ssd does not clear nossd", started
on Mar 31. It has all the answers. :-)

-- 
Hans van Kranenburg
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


remount and conflicting ssd options?

2017-04-15 Thread Chris Murphy
I don't understand this:

/dev/mmcblk0p3 on / type btrfs
(rw,noatime,seclabel,compress=zlib,nossd,ssd_spread,space_cache,commit=150,subvolid=260,subvol=/root)


The fstab uses ssd_spread. It looks like during startup the initial
option is ssd via autodetection and then at switchroot time, when it
goes ro to rw, the fstab options are applied and it becomes
ssd_spread. Fine.

Then later I tried

mount -o remount,ssd

To go back to regular ssd option, but nothing happens, mount still
shows ssd_spread. And then if I do

mount -o remount,nossd

I get the above mount output with nossd,ssd_spread options which would
seem to be a contradiction. At least it's confusing. So... now what?

kernel 4.10.8-200.fc25.x86_64

-- 
Chris Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Btrfs disk layout question

2017-04-15 Thread Chris Murphy
On Sat, Apr 15, 2017 at 12:14 AM, Andrei Borzenkov  wrote:
> 12.04.2017 20:21, Chris Murphy пишет:
>> btrfs-map-logical is the tool that will convert logical to physical
>> and also give what device it's on; but the device notation is copy 1
>> and copy 2, so you have to infer what device that is, it's not
>> explicit.
>>
>
> Actually I just checked with btrfsprogs 4.5.3 on RAID5 and it does print
> OS device name for each data chunk. It does not show checksum chunk for
> RAID56 though.

There is no checksum chunk. The data csums are in their own tree, and
only data is csum'd. The parity strip is contained within a raid56
chunk but is itself not csum'd.

Another thing that I just thought of might be useful is btrfs-debugfs
which is a python script in btrfs-progs, but typically not packaged.
The -f option pointed at a file will show extents with Btrfs logical
bytenr. So that'd be a way to get all the extents, their addresses and
lengths, to then dump into btrfs-map-logical.

For what it's worth, btrfs-map-logical's physical address it reports
is not device LBA. It's a byte offset on the selected device. So if
the device is a partition, the value is the number of bytes from the
start of that partition.



-- 
Chris Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] btrfs-progs: cli-tests: Convert non-raid filesystem to raid

2017-04-15 Thread Lakshmipathi.G
On Mon, Mar 13, 2017 at 06:47:50PM +0100, David Sterba wrote:
> On Sun, Jan 29, 2017 at 08:14:55PM +0530, Lakshmipathi.G wrote:
> > Simple script to verify non-raid filesystem conversion.
> 
> The cli (command line interface) tests are supposed to cover the common
> usecases from the point of option combinations etc, not really verifying
> the result. It would be good if you add more than just one simple test,
> especially for the balance filters. Adding the helpers to prepare the
> filesystem (number of devices, raid profiles) would be a good start.
> Some code for that is in the mkfs tests, but this belongs to the common
> scripts so we could easily use that.

Ok. I'll try to add helpers to common. So that option like this can
be re-used easily.

Cheers.
Lakshmipathi.G
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/12] audit: Use timespec64 to represent audit timestamps

2017-04-15 Thread Arnd Bergmann
On Sat, Apr 8, 2017 at 5:58 PM, Deepa Dinamani  wrote:
>> I have no problem merging this patch into audit/next for v4.12, would
>> you prefer me to do that so at least this patch is merged?
>
> This would be fine.
> But, I think whoever takes the last 2 deletion patches should also take them.
> I'm not sure how that part works out.
>
>> It would probably make life a small bit easier for us in the audit
>> world too as it would reduce the potential merge conflict.  However,
>> that's a relatively small thing to worry about.

As Andrew has picked the remaining patches up into -mm, this will work
out fine: any patches picked up by the respective maintainers for v4.12
should arrive as git pull requests before the -mm patches get applied
at a later stage of the merge window.

 Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] btrfs-progs: misc-tests: Superblock corruption and recovery using backup.

2017-04-15 Thread Lakshmipathi.G
On Mon, Mar 13, 2017 at 06:15:04PM +0100, David Sterba wrote:
> On Fri, Feb 17, 2017 at 04:01:59AM +0530, Lakshmipathi.G wrote:
> > Test script to recover damaged primary superblock using backup superblock.
> > 
> > Signed-off-by: Lakshmipathi.G 
> 
> Thanks for the test, a few comments below.
> 
> > ---
> >  .../019-fix-superblock-corruption/test.sh  | 36 
> > ++
> >  1 file changed, 36 insertions(+)
> >  create mode 100755 tests/misc-tests/019-fix-superblock-corruption/test.sh
> > 
> > diff --git a/tests/misc-tests/019-fix-superblock-corruption/test.sh 
> > b/tests/misc-tests/019-fix-superblock-corruption/test.sh
> > new file mode 100755
> > index 000..95815e4
> > --- /dev/null
> > +++ b/tests/misc-tests/019-fix-superblock-corruption/test.sh
> > @@ -0,0 +1,36 @@
> > +#!/bin/bash
> > +#
> > +# Corrupt primary superblock and restore it using backup superblock.
> > +#
> > +
> > +source $TOP/tests/common
> > +
> > +check_prereq btrfs-select-super
> > +check_prereq btrfs
> > +
> > +setup_root_helper
> > +prepare_test_dev 512M
> > +
> > +FIRST_SUPERBLOCK_OFFSET=65536
> > +
> > +test_superblock_restore()
> > +{
> > +   run_check $SUDO_HELPER $TOP/mkfs.btrfs -f $TEST_DEV
> > +
> > +   # Corrupt superblock checksum
> > +dd if=/dev/zero of=$TEST_DEV seek=$FIRST_SUPERBLOCK_OFFSET bs=1 \
> > +count=4  conv=notrunc &> /dev/null
> > +   # Run btrfs check to detect corruption
> > +   $TOP/btrfs check $TEST_DEV >& /dev/null && \
> > +   _fail "btrfs check should detect corruption"
> 
> No run_check? In general all test output is desired and should be in the
> test log so you can use 'run_mayfail'. Also, please keep the entier
> output and don't redirect it to /dev/null .
> 
> > +   # Copy backup superblock to primary
> > +   run_check $TOP/btrfs-select-super -s 1 $TEST_DEV
> > +
> > +   echo "Performing btrfs check" &>> $RESULTS
> 
> Use _log for that
> 
> > +   $TOP/btrfs check $TEST_DEV &>> $RESULTS
> 
> Use run_check
> 
> > +if [ $? -ne 0 ]; then
> > +   _fail "Failed to fix superblock."
> 
> And this would be unnecessary.
> 
> I've added a section to tests/README.md to describe the common coding
> practices. Feel free to read/update/fix. Older tests could violate the
> recommendations so at least new tests are in line and we'll update the
> rest incrementally.

Just sent a new patch(v4), addressing above review comments.thanks.

Cheers.
Lakshmipathi.G


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4] btrfs-progs: misc-tests: Superblock corruption and recovery using backup

2017-04-15 Thread Lakshmipathi.G
Signed-off-by: Lakshmipathi.G 
---
 .../020-fix-superblock-corruption/test.sh  | 35 ++
 1 file changed, 35 insertions(+)
 create mode 100755 tests/misc-tests/020-fix-superblock-corruption/test.sh

diff --git a/tests/misc-tests/020-fix-superblock-corruption/test.sh 
b/tests/misc-tests/020-fix-superblock-corruption/test.sh
new file mode 100755
index 000..0faebad
--- /dev/null
+++ b/tests/misc-tests/020-fix-superblock-corruption/test.sh
@@ -0,0 +1,35 @@
+#!/bin/bash
+#
+# Corrupt primary superblock and restore it using backup superblock.
+#
+
+source $TOP/tests/common
+
+check_prereq btrfs-select-super
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+FIRST_SUPERBLOCK_OFFSET=65536
+
+test_superblock_restore()
+{
+   run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+
+   # Corrupt superblock checksum
+run_check $SUDO_HELPER dd if=/dev/zero of="$TEST_DEV" \
+   seek="$FIRST_SUPERBLOCK_OFFSET" bs=1 count=4 conv=notrunc
+
+   # Run btrfs check to detect corruption
+   run_mayfail "$TOP/btrfs" check "$TEST_DEV" && \
+   _fail "btrfs check should detect corruption"
+
+   # Copy backup superblock to primary
+   run_check "$TOP/btrfs-select-super" -s 1 "$TEST_DEV"
+
+   #Performing btrfs check
+   run_check "$TOP/btrfs" check "$TEST_DEV"
+}
+
+test_superblock_restore
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4] btrfs-progs: fsck-tests: verify 'btrfs check --repair' fixes corrupted nlink field

2017-04-15 Thread Lakshmipathi.G
commit 5cdcc57b8d599f03fe692e0285ee9e17f32267fa
Author: Lakshmipathi.G 
Date:   Sat Apr 15 14:26:20 2017 +0530

btrfs-progs: fsck-tests: verify 'btrfs check --repair' fixes corrupted 
nlink field

Signed-off-by: Lakshmipathi.G 

diff --git a/tests/fsck-tests/026-check-inode-link/test.sh 
b/tests/fsck-tests/026-check-inode-link/test.sh
new file mode 100755
index 000..3a7a3c1
--- /dev/null
+++ b/tests/fsck-tests/026-check-inode-link/test.sh
@@ -0,0 +1,38 @@
+#!/bin/bash
+# verify that 'btrfs check --repair' fixes corrupted inode nlink field
+
+source $TOP/tests/common
+
+check_prereq btrfs-corrupt-block
+check_prereq mkfs.btrfs
+
+setup_root_helper
+prepare_test_dev
+
+test_inode_nlink_field()
+{
+   run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+
+   run_check_mount_test_dev
+   run_check $SUDO_HELPER touch "$TEST_MNT/test_nlink.txt"
+
+   # find inode_number
+   inode_number=`stat -c%i "$TEST_MNT/test_nlink.txt"`
+   run_check_umount_test_dev
+
+   # corrupt nlink field of inode object
+run_check $SUDO_HELPER "$TOP/btrfs-corrupt-block" -i "$inode_number" \
+   -f nlink "$TEST_DEV"
+
+   check_image "$TEST_DEV"
+
+   # verify link count
+   run_check_mount_test_dev
+   link_count=`stat -c%h "$TEST_MNT/test_nlink.txt"`
+   run_check_umount_test_dev
+   if [ "$link_count" != 1 ];then
+   _fail "btrfs check --repair: Unable to fix nlink corruption."
+   fi
+}
+
+test_inode_nlink_field
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] btrfs-progs: fsck-tests: verify 'btrfs check --repair' fixes corrupted nlink field

2017-04-15 Thread Lakshmipathi.G
On Thu, Mar 30, 2017 at 05:52:11PM +0200, David Sterba wrote:
> On Thu, Feb 02, 2017 at 01:35:02PM +0530, Lakshmipathi.G wrote:
> > Signed-off-by: Lakshmipathi.G 
> > ---
> >  tests/fsck-tests/026-check-inode-link/test.sh | 30 
> > +++
> >  1 file changed, 30 insertions(+)
> >  create mode 100755 tests/fsck-tests/026-check-inode-link/test.sh
> > 
> > diff --git a/tests/fsck-tests/026-check-inode-link/test.sh 
> > b/tests/fsck-tests/026-check-inode-link/test.sh
> > new file mode 100755
> > index 000..6822ee2
> > --- /dev/null
> > +++ b/tests/fsck-tests/026-check-inode-link/test.sh
> > @@ -0,0 +1,30 @@
> > +#!/bin/bash
> > +# verify that 'btrfs check --repair' fixes corrupted inode nlink field
> > +
> > +source $TOP/tests/common
> > +
> > +check_prereq btrfs-corrupt-block
> > +check_prereq mkfs.btrfs
> > +
> > +setup_root_helper
> > +prepare_test_dev 512M
> 
> Please use default size unless you really need 512M for the test
> purposes.
> 
> > +
> > +test_inode_nlink_field()
> > +{
> > +   run_check $SUDO_HELPER $TOP/mkfs.btrfs -f $TEST_DEV
> > +
> > +   run_check_mount_test_dev
> > +   run_check $SUDO_HELPER touch $TEST_MNT/test_nlink.txt
> > +
> > +   # find inode_number
> > +   inode_number=`stat -c%i $TEST_MNT/test_nlink.txt`
> > +   run_check_umount_test_dev
> > +
> > +   # corrupt nlink field of inode object
> > +run_check $SUDO_HELPER $TOP/btrfs-corrupt-block -i $inode_number \
> > +   -f nlink $TEST_DEV
> > +
> > +   check_image $TEST_DEV
> 
> The wrapper runs a pre-check that must detect errors, then runs --repair
> and then again a plain check. Is this intended here?
> 
Yes its intentional. The test should fail, if its unable to detect the 
corruption.
This is to ensure btrfs-corrupt-block -f nlink works as expected.
> I think we should try to mount the fixed image again and read the file
> or check link count.
> 
Right, I missed the verification part, will added this and change 
prepare_test_dev 
to default size.
> Please add shell quotation around all variables.
Now quotation is added.

Cheers.
Lakshmipathi.G
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Btrfs disk layout question

2017-04-15 Thread Andrei Borzenkov
12.04.2017 20:21, Chris Murphy пишет:
> btrfs-map-logical is the tool that will convert logical to physical
> and also give what device it's on; but the device notation is copy 1
> and copy 2, so you have to infer what device that is, it's not
> explicit.
> 

Actually I just checked with btrfsprogs 4.5.3 on RAID5 and it does print
OS device name for each data chunk. It does not show checksum chunk for
RAID56 though.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html