Re: [Linux-nvdimm] [PATCH] pmem: Add prints at module load and unload

On 04/02/2015 06:39 PM, Dan Williams wrote: > On Thu, Apr 2, 2015 at 8:31 AM, Boaz Harrosh <boaz(a)plexistor.com&gt; wrote: >> Hi Christoph, Ingo >> >> Please consider this small patch below just a small print at module >> load/unload so to know at user systems how things progressed. >> As it is now, we know nothing. For any other disk kind we have two >> tuns of prints. >> >> --- >> From: Boaz Harrosh <boaz(a)plexistor.com&gt; >> Date: Thu, 2 Apr 2015 16:43:48 +0300 >> Subject: [PATCH] pmem: Add prints at module load and unload >> >> When debugging people's systems it is helpful >> to see what went on. The load and unload of pmem is >> an important event. >> >> The importance is the number of loaded devices and >> error status. The exact device's addresses created >> we can see from the other prints at e820 so no need >> to duplicate this information. >> >> While at it fix up a small issue with rw flags. > > Separate patch for fixes please. Sigh, OK I was preparing this and hopping it would just be SQUASHED into the original patch but Ingo bit me to it and already submitted the patches. > >> >> Signed-off-by: Boaz Harrosh <boaz(a)plexistor.com&gt; >> --- >> drivers/block/pmem.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c >> index 988f384..3f15fbc 100644 >> --- a/drivers/block/pmem.c >> +++ b/drivers/block/pmem.c >> @@ -91,8 +91,9 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector, >> { >> struct pmem_device *pmem = bdev->bd_disk->private_data; >> >> + rw &= WRITE; >> pmem_do_bvec(pmem, page, PAGE_CACHE_SIZE, 0, rw, sector); >> - page_endio(page, rw & WRITE, 0); >> + page_endio(page, rw, 0); >> >> return 0; >> } >> @@ -248,6 +249,9 @@ static int __init pmem_init(void) >> error = platform_driver_register(&pmem_driver); >> if (error) >> unregister_blkdev(pmem_major, "pmem"); >> + >> + pr_info("pmem: init %d devices => %d\n", >> + atomic_read(&pmem_index), error); > > If anything I think these should be dev_dbg(). We do not have a dev at any of this point, and it does not belong to any specific device.

Also I would like this _info and not _dbg so to always have it, also for production. See the chatter for a single SCSI disk the minimum we need is just the small print that tells all that we need (for now)