Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <ed3c5450-3dce-4f6a-9a8c-04fcdba1cbf2@kernel.dk>
Date:   Fri, 27 Oct 2023 10:06:33 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: lockdep: holding locks across syscall boundaries
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Ingo Molnar <mingo@redhat.com>, LKML <linux-kernel@vger.kernel.org>
References: <a99a7fbe-ec31-4e31-87c7-1b7ae1dd1a5a@kernel.dk>
 <20231027155949.GA26550@noisy.programming.kicks-ass.net>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <20231027155949.GA26550@noisy.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/27/23 9:59 AM, Peter Zijlstra wrote:
> On Fri, Oct 27, 2023 at 09:14:53AM -0600, Jens Axboe wrote:
>> Hi,
>>
>> Normally we'd expect locking state to be clean and consistent across
>> syscall entry and exit, as that is always the case for sync syscalls.
> 
>> We currently have a work-around for holding a lock from aio, see
>> kiocb_start_write(), which pretends to drop the lock from lockdeps
>> perspective, as it's held from submission to until kiocb_end_write() is
>> called at completion time.
> 
> I was not aware of this, the only such hack I knew about was the
> filesystem freezer thing.
> 
> The problem with holding locks past the end of a syscall is that you'll
> nest whatever random lock hierarchies possibly by every other syscall
> under that lock.

Can you expand on that bit, not quite sure I follow. Do we reset the
locking dependencies between syscalls?

>> This is a bit of an ugly work-around, and defeats the purpose of
>> lockdep.
>>
>> Since I've now got another case where I want to hold a resource across
>> syscalls, is there a better way to do this?
>>
>> This is for inode_dio_start(), which increments an inode int count, and
>> inode_dio_end() which decrements it. If a task is doing
>> inode_dio_start() and then inode_dio_wait(), I want to trigger this. I
>> have a hack that does this, but it disables lockdep_sys_exit() as
>> otherwise I just get that warning rather than the more useful one.
> 
> Suppose syscall-a returns with your kiocb thing held, call it lock A
> Suppose syscall-b returns with your inode thing held, call it lock B
> 
> Then userspace does:
> 
> 	syscall-a
> 	syscall-b
> 
> while it also does:
> 
> 	syscall-b
> 	syscall-a
> 
> and we're up a creek, no?

Should this not get caught by the usual lock ordering rules? Because
that is obviously a bug, ordering would have to be consistent, just like
if we have:

	syscall-a
		lock(a);
		lock(b);

	syscall-b
		lock(b);
		lock(a)

-- 
Jens Axboe