Re: [PATCH A+B] pmem: Add prints at module load and unload

From: Greg KH <gregkh@linuxfoundation.org>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: Christoph Hellwig <hch@lst.de>,
	linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, x86@kernel.org, axboe@kernel.dk,
	ross.zwisler@linux.intel.com
Subject: Re: [PATCH A+B] pmem: Add prints at module load and unload
Date: Mon, 13 Apr 2015 14:36:58 +0200	[thread overview]
Message-ID: <20150413123658.GA28718@kroah.com> (raw)
In-Reply-To: <552BB107.2070707@plexistor.com>

On Mon, Apr 13, 2015 at 03:05:27PM +0300, Boaz Harrosh wrote:
> On 04/13/2015 12:05 PM, Greg KH wrote:
> > On Tue, Apr 07, 2015 at 06:46:15PM +0300, Boaz Harrosh wrote:
> >> Hi Christoph, Ingo
> >>
> >> It is important in the lab for postmortem analysis to know if
> >> pmem driver loaded and/or unloaded. And the return code from this
> >> operation.
> >>
> >> I submit two versions [A] more chatty and version [B]. Both give me
> >> the info I need.
> >>
> >> I like [B] because [A] prints more lines, and also the driver might not
> >> load at the end and we would still not see it from [A]'s prints.
> >>
> >> But it does not matter that much just take any one you guys like
> >> better.
> >>
> >> Here are the commit logs:
> >> ---
> >> [PATCH 1A] pmem: Add prints at pmem_probe/remove
> >>
> >> Add small prints at creation/remove of pmem devices.
> >> So we can see in dmesg logs when users loaded/unloaded
> >> the pmem driver and what devices were created.
> >>
> >> The prints will look like this:
> >> Printed by e820 on load:
> >>  [  +0.000000] user: [mem 0x0000000100000000-0x000000015fffffff] persistent (type 12)
> >>  [  +0.000000] user: [mem 0x0000000160000000-0x00000001dfffffff] persistent (type 12)
> >>  ...
> >> Printed by modprobe pmem:
> >>  [  +0.003065] pmem pmem.0.auto: probe [0x0000000100000000:0x60000000]
> >>  [  +0.001816] pmem pmem.1.auto: probe [0x0000000160000000:0x80000000]
> >>  ...
> >> Printed by modprobe -r pmem:
> >>  [ +16.299145] pmem pmem.1.auto: remove
> >>  [  +0.011155] pmem pmem.0.auto: remove
> >>
> >> Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
> > 
> > Don't polute the kernel logs with "chatty" things like this, 
> 
> Why do you say this is chatty. With [B] This is a single line of print
> on modprobe. With [A] it is a print per device (Is why I like [B])
> Compare to all the other block-devices in the system, say scsi, that print
> bunch of info for each device, this is very very minimalistic.
> 
> > just
> > trigger off of the block device uevent if you really want to know if the
> > block device is still around or not.
> > 
> 
> Again I do not need this for run time. At run time I have two tons of ways
> to check and see. BTW a uevent is already triggered for insertion as part
> of regular block core operation.
> 
> I need this at dmesg for when analyzing users logs, say when a crash happens.
> I need to see what/when drivers were loaded/unloaded. It is common practice
> in dmseg for block devices to leave foot prints.
> 
> Sigh, do you not believe that this single line in dmseg makes my life much
> easier?

it might, as grepping kernel logs is a "lazy" way of doing things.  We
are working hard to make it so that each and every device found in the
system does _not_ print out kernel log messages, as they really are
pretty pointless and useless overall for the 99.99% of the time.

Anyway, I'm not the maintainer here of this driver, so I don't have the
final say, but really, if you are relying on kernel log messages for
specific things to happen in your system, you are doing it wrong as they
can change and disappear in any future kernel release, they are NOT an
api.

thanks,

greg k-h