Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180126193954.GB13338@ZenIV.linux.org.uk>
References: <20180126024649.200330-1-joelaf@google.com> <20180126031346.GW13338@ZenIV.linux.org.uk>
 <CAJWu+oo+Y4eNTX+vMceYTB-FF5A190OHP7yPKkDGYH-jTqSsmQ@mail.gmail.com> <20180126193954.GB13338@ZenIV.linux.org.uk>
From:   Joel Fernandes <joelaf@google.com>
Date:   Fri, 26 Jan 2018 12:45:34 -0800
Message-ID: <CAJWu+opST5wy=X9J0dhU6w4tByk5wxmUO=K5p5bewNJjuzERyg@mail.gmail.com>
Subject: Re: [PATCH] ashmem: Fix lockdep issue during llseek
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     LKML <linux-kernel@vger.kernel.org>, Todd Kjos <tkjos@google.com>,
        Arve Hjonnevag <arve@android.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Al,

On Fri, Jan 26, 2018 at 11:39 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
[..]
>
>> But one usecase for the mutex is with concurrent lseeks, you can end
>> up with a file->f_pos that is different from the latest update to
>> asma->file->f_pos. A barrier could fix this it too though. Any
>> thoughts?
>
> lseek(2) is serialized against lseek(2) and read(2) on the same struct
> file - see fdget_pos() for details.

Ah right, Ok. Thanks.

> ashmem_mutex really does look like an overkill - something much lighter
> should serve just fine...

There's also the issue of asma->size getting updated from while being
in read ashmem_llseek (although this is a theoretical concern):

if (asma->size == 0) {
  ret = -EINVAL;
  goto out;
}

Which could just use READ_ONCE and WRITE_ONCE instead of the mutex I suppose?

Other than that, I don't see any other issues at the moment with
dropping of the ashmem_mutex from ashmem_llseek.

thanks,

- Joel