Received: by 2002:a25:1506:0:0:0:0:0 with SMTP id 6csp358517ybv; Thu, 13 Feb 2020 01:43:21 -0800 (PST) X-Google-Smtp-Source: APXvYqysDv6tQ2c9gZKPJM63UcxX3CffzvRLwg4cQyOhjGzDFfo3swIyjTB5Jc4gWqLkN83h7Dls X-Received: by 2002:a9d:6c06:: with SMTP id f6mr13030691otq.318.1581587000968; Thu, 13 Feb 2020 01:43:20 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1581587000; cv=none; d=google.com; s=arc-20160816; b=bXetljUHbAil5ZsgIj8bCPrBPaB98CJYgJDcAeJp6iEHde6DokJGIv58KRGQ2VPhxX UXtIipaIKSFVptp1RSRjLVYMPDMobG7mmPZsKU60BABhwH2SIesyIbw3ihhnk5H2zcXB VU2m0ZksW7LcCKbvCb823S0N2bpS+l2AMrRISZdQH7u7iC6bq2o5eNrDXJpftqWloD9L 6HnKCohXKSGwMbB5pHNtOhS5q4luUQpxWEDUMBoM33V9ZB/RglywBXu8S6VwicRVsAM/ 7uIE8GVTatehYQlO5LFcKUdjGV3kBvHYVIzl3SdFtU8fFMQK1/lMqq+Nf//QqXmuZDxB jkyQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:user-agent:message-id :in-reply-to:date:references:subject:cc:to:from; bh=7HppTs64I8QaaW1CR0j+KhxLas121iYrboig3Uj/hmg=; b=iqhX2r0ejVfRyQqFTF65LCVrzc1faNKYNLkUKi4og8H+dW233CZ3uoyeOo46+/ztbN lvYyZncGZ1201bysTts7zCzoQmYNfJsFWyh8i7JrIqfyoqqsQOe/biDF/xpBXuD+gFJa bKRxDnokPiCIiill6tMjaxSw/bK1co6y8Zhm/NNjfk6U54HIRoZ49MfsOZrSB7SV/+G6 1zK+cvjRQ30tHCAHp0lXVv15Yj7exR6fG7pYR6fXHXa8+GF06yW1B+7dFexXGr2x/8fk JGf2pFdshZtyBd9fph/FfvztSdAhhG+PHVPNWf8I8fvwYEc1Ny/pfYsFHPXlbG9pmixG YuCg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b10si834547oic.153.2020.02.13.01.43.08; Thu, 13 Feb 2020 01:43:20 -0800 (PST) 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; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729709AbgBMJml (ORCPT + 99 others); Thu, 13 Feb 2020 04:42:41 -0500 Received: from Galois.linutronix.de ([193.142.43.55]:51294 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729440AbgBMJml (ORCPT ); Thu, 13 Feb 2020 04:42:41 -0500 Received: from localhost ([127.0.0.1] helo=vostro.local) by Galois.linutronix.de with esmtp (Exim 4.80) (envelope-from ) id 1j2B13-0006Ng-1r; Thu, 13 Feb 2020 10:42:37 +0100 From: John Ogness To: Sergey Senozhatsky Cc: Petr Mladek , Peter Zijlstra , Sergey Senozhatsky , Steven Rostedt , Linus Torvalds , Greg Kroah-Hartman , Andrea Parri , Thomas Gleixner , kexec@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] printk: use the lockless ringbuffer References: <20200128161948.8524-1-john.ogness@linutronix.de> <20200128161948.8524-3-john.ogness@linutronix.de> <20200213090757.GA36551@google.com> Date: Thu, 13 Feb 2020 10:42:35 +0100 In-Reply-To: <20200213090757.GA36551@google.com> (Sergey Senozhatsky's message of "Thu, 13 Feb 2020 18:07:57 +0900") Message-ID: <87v9oarfg4.fsf@linutronix.de> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2020-02-13, Sergey Senozhatsky wrote: >> - while (user->seq == log_next_seq) { >> + if (!prb_read_valid(prb, user->seq, r)) { >> if (file->f_flags & O_NONBLOCK) { >> ret = -EAGAIN; >> logbuf_unlock_irq(); >> @@ -890,30 +758,26 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf, >> >> logbuf_unlock_irq(); >> ret = wait_event_interruptible(log_wait, >> - user->seq != log_next_seq); >> + prb_read_valid(prb, user->seq, r)); >> if (ret) >> goto out; >> logbuf_lock_irq(); >> } >> >> - if (user->seq < log_first_seq) { >> - /* our last seen message is gone, return error and reset */ >> - user->idx = log_first_idx; >> - user->seq = log_first_seq; >> + if (user->seq < r->info->seq) { >> + /* the expected message is gone, return error and reset */ >> + user->seq = r->info->seq; >> ret = -EPIPE; >> logbuf_unlock_irq(); >> goto out; >> } > > Sorry, why doesn't this do something like > > if (user->seq < prb_first_seq(prb)) { > /* the expected message is gone, return error and reset */ > user->seq = prb_first_seq(prb); > ret = -EPIPE; > ... > } Here prb_read_valid() was successful, so a record _was_ read. The kerneldoc for the prb_read_valid() says: * On success, the reader must check r->info.seq to see which record was * actually read. The value will either be the requested user->seq or some higher value because user->seq is not available. There are 2 reasons why user->seq is not available (and a later record _is_ available): 1. The ringbuffer overtook user->seq. In this case, comparing and then setting using prb_first_seq() could be appropriate. And r->info->seq might even already be what prb_first_seq() would return. (More on this below.) 2. The record with user->seq has no data because the writer failed to allocate dataring space. In this case, resetting back to prb_first_seq() would be incorrect. And since r->info->seq is the next valid record, it is appropriate that the next devkmsg_read() starts there. Rather than checking these cases separately, it is enough just to check for the 2nd case. For the 1st case, prb_first_seq() could be less than r->info->seq if all the preceeding records have no data. But this just means the whole set of records with missing data are skipped, which matches existing behavior. (For example, currently when devkmsg is behind 10 messages, there are not 10 -EPIPE returns. Instead it immediately catches up to the next available record.) Perhaps the new comment should be: /* * The expected message is gone, return error and * reset to the next available message. */ John Ogness