Re: [Linux-nvdimm] [PATCH] pmem: Add prints at module load and unload
From: Dan Williams
Date: Thu Apr 02 2015 - 12:01:22 EST
On Thu, Apr 2, 2015 at 8:47 AM, Boaz Harrosh <boaz@xxxxxxxxxxxxx> wrote:
> On 04/02/2015 06:39 PM, Dan Williams wrote:
>> On Thu, Apr 2, 2015 at 8:31 AM, Boaz Harrosh <boaz@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>> 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@xxxxxxxxxxxxx>
>>> 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.
Ah, true this is prior to the driver attaching... that said it seems
more relevant to print from probe() (where we do have a device) than
init where the device may remain idle due to some other policy.
> 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)
Not sure we want to follow so closely in the footsteps of SCSI's chattiness.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/