From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752007AbdINL3P (ORCPT ); Thu, 14 Sep 2017 07:29:15 -0400 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:54869 "EHLO out3-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751413AbdINL3N (ORCPT ); Thu, 14 Sep 2017 07:29:13 -0400 X-ME-Sender: X-Sasl-enc: xfVH677S14KDXvYdIm16p1aTLPFHOrfKiRmza+R4m1UB 1505388552 Subject: Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation To: skinsbursky@virtuozzo.com Cc: autofs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org, ldv@altlinux.org References: <20170901110906.2669.1577.stgit@localhost.localdomain> <20170901112144.2669.1453.stgit@localhost.localdomain> From: Ian Kent Message-ID: Date: Thu, 14 Sep 2017 19:29:08 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/09/17 17:24, Stanislav Kinsburskiy wrote: > > > 14.09.2017 02:38, Ian Kent пишет: >> On 01/09/17 19:21, Stanislav Kinsburskiy wrote: >>> Signed-off-by: Stanislav Kinsburskiy >>> --- >>> fs/autofs4/autofs_i.h | 3 +++ >>> fs/autofs4/dev-ioctl.c | 3 +++ >>> fs/autofs4/inode.c | 4 +++- >>> 3 files changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h >>> index 4737615..3da105f 100644 >>> --- a/fs/autofs4/autofs_i.h >>> +++ b/fs/autofs4/autofs_i.h >>> @@ -120,6 +120,9 @@ struct autofs_sb_info { >>> struct list_head active_list; >>> struct list_head expiring_list; >>> struct rcu_head rcu; >>> +#ifdef CONFIG_COMPAT >>> + unsigned is32bit:1; >>> +#endif >>> }; >>> >>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb) >>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c >>> index b7c816f..467d6c4 100644 >>> --- a/fs/autofs4/dev-ioctl.c >>> +++ b/fs/autofs4/dev-ioctl.c >>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp, >>> sbi->pipefd = pipefd; >>> sbi->pipe = pipe; >>> sbi->catatonic = 0; >>> +#ifdef CONFIG_COMPAT >>> + sbi->is32bit = is_compat_task(); >>> +#endif >>> } >>> out: >>> put_pid(new_pid); >>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c >>> index 09e7d68..21d3c0b 100644 >>> --- a/fs/autofs4/inode.c >>> +++ b/fs/autofs4/inode.c >>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent) >>> } else { >>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID); >>> } >>> - >>> +#ifdef CONFIG_COMPAT >>> + sbi->is32bit = is_compat_task(); >>> +#endif >>> if (autofs_type_trigger(sbi->type)) >>> __managed_dentry_set_managed(root); >>> >>> >> >> Not sure about this. >> >> Don't you think it would be better to avoid the in code #ifdefs by doing some >> checks and defines in the header file and defining what's need to just use >> is_compat_task(). >> > > Yes, might be... > >> Not sure 2 patches are needed for this either ...... >> > > Well, I found this issue occasionally. I'm wondering what the symptoms are? > And, frankly speaking, it's not clear to me, whether this issue is important at all, so I wanted to clarify this first. > Thanks to O_DIRECT, the only way to catch the issue is to try to read more, than expected, in compat task (that's how I found it). Right, the O_DIRECT patch from Linus was expected to fix the structure alignment problem. The stuct field offsets are ok aren't they? > I don't see any other flaw so far. And if so, that, probably, we shouldn't care about the issue at all. > What do you think? If we are seeing hangs, incorrect struct fields or similar something should be done about it but if all is actually working ok then the O_DIRECT fix is doing it's job and further changes aren't necessary. Ian