| Message ID | 20180416191533.GZ21272@twin.jikos.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 16 Apr 2018, at 21:15, David Sterba <dsterba@suse.cz> wrote: > On Mon, Apr 16, 2018 at 09:55:45PM +0200, René Rebe wrote: >> Hi, >> >> On 04/16/2018 06:48 PM, David Sterba wrote: >>> The warnings are valid, there's unaligned access introduced by patch >>> >>> 23b5ec74943f44378b68c0edd8e210a86318ea5e >>> btrfs: fix readdir deadlock with pagefault >>> >>> The directory entries (struct dir_entry) are copied to a temporary >>> buffer as they fit, ie. no alignment, and the members accessed in >>> several places. >>> >>> The following patch adds the proper unaligned access, only compile-tested. >>> Please test and let me know, thanks! >> Would have loved to immediately give it a try, however, sorry, >> I forgot to mention I'm on the latest stable release -4.16.2- >> on a first glance this does not look like it does just apply. >> >> I would re-base myself if I would not also have a glibc initialization >> bug to hunt and debug, too :-/ >> >> If you happen to also rebase it for current -stable, ... ;-) > > Sure, attached a 4.16.2 version. > <0001-test-readdir-unaligned-access.patch> Cool, thanks, built and so far it works, the warnings are gone. https://www.youtube.com/watch?v=XYsKct4T2xk Greetings form Berlin, René
On Tue, Apr 17, 2018 at 12:18:45PM +0200, René Rebe wrote: > On 16 Apr 2018, at 21:15, David Sterba <dsterba@suse.cz> wrote: > > On Mon, Apr 16, 2018 at 09:55:45PM +0200, René Rebe wrote: > >> On 04/16/2018 06:48 PM, David Sterba wrote: > >>> The warnings are valid, there's unaligned access introduced by patch > >>> > >>> 23b5ec74943f44378b68c0edd8e210a86318ea5e > >>> btrfs: fix readdir deadlock with pagefault > >>> > >>> The directory entries (struct dir_entry) are copied to a temporary > >>> buffer as they fit, ie. no alignment, and the members accessed in > >>> several places. > >>> > >>> The following patch adds the proper unaligned access, only compile-tested. > >>> Please test and let me know, thanks! > >> Would have loved to immediately give it a try, however, sorry, > >> I forgot to mention I'm on the latest stable release -4.16.2- > >> on a first glance this does not look like it does just apply. > >> > >> I would re-base myself if I would not also have a glibc initialization > >> bug to hunt and debug, too :-/ > >> > >> If you happen to also rebase it for current -stable, ... ;-) > > > > Sure, attached a 4.16.2 version. > > <0001-test-readdir-unaligned-access.patch> > > Cool, thanks, built and so far it works, the warnings are gone. > > https://www.youtube.com/watch?v=XYsKct4T2xk Thanks for testing on a real hardware! I'm not aware of regular runtime testing on sparc, the other architectures with strict alignment requirement have probably the emulation turned on by default so the warnings do not appear. Or nobody bothers to report them though the fixes are typically simple. I like your vintage hw videos, keep going! -- 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/inode.c b/fs/btrfs/inode.c index c7b75dd58fad..c2df7b158820 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -44,6 +44,7 @@ #include <linux/uio.h> #include <linux/magic.h> #include <linux/iversion.h> +#include <asm/unaligned.h> #include "ctree.h" #include "disk-io.h" #include "transaction.h" @@ -5951,11 +5952,13 @@ static int btrfs_filldir(void *addr, int entries, struct dir_context *ctx) struct dir_entry *entry = addr; char *name = (char *)(entry + 1); - ctx->pos = entry->offset; - if (!dir_emit(ctx, name, entry->name_len, entry->ino, - entry->type)) + ctx->pos = get_unaligned(&entry->offset); + if (!dir_emit(ctx, name, get_unaligned(&entry->name_len), + get_unaligned(&entry->ino), + get_unaligned(&entry->type))) return 1; - addr += sizeof(struct dir_entry) + entry->name_len; + addr += sizeof(struct dir_entry) + + get_unaligned(&entry->name_len); ctx->pos++; } return 0; @@ -6045,14 +6048,15 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx) } entry = addr; - entry->name_len = name_len; + put_unaligned(name_len, &entry->name_len); name_ptr = (char *)(entry + 1); read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1), name_len); - entry->type = btrfs_filetype_table[btrfs_dir_type(leaf, di)]; + put_unaligned(btrfs_filetype_table[btrfs_dir_type(leaf, di)], + &entry->type); btrfs_dir_item_key_to_cpu(leaf, di, &location); - entry->ino = location.objectid; - entry->offset = found_key.offset; + put_unaligned(location.objectid, &entry->ino); + put_unaligned(found_key.offset,&entry->offset); entries++; addr += sizeof(struct dir_entry) + name_len; total_len += sizeof(struct dir_entry) + name_len;