From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762975AbbA3SUR (ORCPT ); Fri, 30 Jan 2015 13:20:17 -0500 Received: from smtpout.karoo.kcom.com ([212.50.160.34]:2433 "EHLO smtpout.karoo.kcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751735AbbA3SUP (ORCPT ); Fri, 30 Jan 2015 13:20:15 -0500 X-IronPort-AV: E=Sophos;i="5.09,492,1418083200"; d="scan'208";a="110737477" Date: Fri, 30 Jan 2015 18:20:12 +0000 From: Mike Crowe To: Andrew Morton , Kay Sievers Cc: linux-kernel@vger.kernel.org, Arnd Bergmann , Greg Kroah-Hartman Subject: Re: kmsg: lseek errors confuse glibc's dprintf Message-ID: <20150130182012.GA26310@mcrowe.com> References: <20150115173132.GA7486@mcrowe.com> <20150123150939.f96d28a380fc5ec1f3894d3a@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150123150939.f96d28a380fc5ec1f3894d3a@linux-foundation.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 23 January 2015 at 15:09:39 -0800, Andrew Morton wrote: > On Thu, 15 Jan 2015 17:31:32 +0000 Mike Crowe wrote: > > > glibc's dprintf implementation does not work correctly with /dev/kmsg file > > descriptors because glibc treats receiving EBADF and EINVAL from lseek when > > trying to determine the current file position as errors. See > > https://sourceware.org/bugzilla/show_bug.cgi?id=17830 > > > > From what I can tell prior to Kay Sievers printk record commit > > e11fea92e13fb91c50bacca799a6131c81929986, calling lseek(fd, 0, SEEK_CUR) > > with such a file descriptor would not return an error. > > > > Prior to Kay's change, Arnd Bergmann's commit > > 6038f373a3dc1f1c26496e60b6c40b164716f07e seemed to go to some lengths to > > preserve the successful return code rather than returning (the perhaps more > > logical) -ESPIPE. > > > > glibc is happy with either a successful return or -ESPIPE. > > > > For maximum compatibility it seems that success should be returned but > > given Kay's new seek interface perhaps this isn't helpful. > > > > This patch ensures that such a seek succeeds: > > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index 02d6b6d..b3ff6f0 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -693,7 +693,7 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence) > > loff_t ret = 0; > > > > if (!user) > > - return -EBADF; > > + return (whence == SEEK_CUR) ? 0 : -EBADF; > > What's actually going on here? What is the significance of > file->private_data==NULL and why does this code treat it as an error? Kay is presumably the expert on this but my understanding is that opening /dev/kmsg for writing only is supposed to be as lightweight as possible - there's not even a context structure so file->private_data is NULL (see devmksg_open.) In this mode seeking is not supported. I believe that EBADF is not a particularly helpful error in this case but didn't want to complicate the patch with a separate change. When /dev/kmsg is opened for reading seeking is supported but the seek behaviour is tailored to (I assume) systemd journal's usage. > > if (offset) > > return -ESPIPE; > > > > @@ -718,6 +718,11 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence) > > user->idx = log_next_idx; > > user->seq = log_next_seq; > > break; > > + case SEEK_CUR: > > + /* For compatibility with userspace requesting the > > + * current file position. */ > > + ret = 0; > > + break; > > Can we actually do something useful here? Return some value which can > be fed back into SEEK_SET to restore the file position? Perhaps that would be useful for someone. It would certainly be more logical than just returning zero. > > > default: > > ret = -EINVAL; > > } Mike.