Re: [PATCH 1/2] btrfs-progs: fsck: Add support to clear v1 free space cache.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





At 10/25/2016 10:09 PM, David Sterba wrote:
On Thu, Oct 13, 2016 at 05:22:26PM +0800, Qu Wenruo wrote:
Kernel clear_cache mount option will only rebuilt free space cache if
used space of that chunk has changed.

So it won't ensure any corrupted free space cache get cleared.

So add a new option "--clear-space-cache v1|v2" to btrfsck, to
completely wipe out free space cache.
So kernel won't complain again.

Reported-by: Ivan P <chrnosphered@xxxxxxxxx>
Signed-off-by: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>

Applied with the attached diff, use it as a review feedback.


Thanks for the diff, quite helpful.

But I'm a little concerned about the minor coding styling.

1) For non-zero comparison
-				if (strcmp(optarg, "v1")) {
+				if (strcmp(optarg, "v1") != 0) {

Is it recommended to use "!= 0" other than hiding it?
I often see code hiding the "!= 0", so I'm a little curious about this.

2) Long string indent
-			error("failed to locate free space cache extent for block group %llu",
-			      bg->key.objectid);
+			error(
+	"failed to locate free space cache extent for block group %llu: %d",
+				bg->key.objectid, ret);

Kernel coding style allows us to use string longer than 80 chars, so we don't need to break long string.

But I'm not sure if we should change the indent.
Normally I just keep the string at the same line of error()/warning(), as we have no 80 chars limit.

Or we should change indent to allow them fit in 80 chars?
IIRC I did it several times in kernel or btrfs-progs, but it seems to be modified to same line version.

Thanks,
Qu


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




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux