lkml.org 
[lkml]   [2021]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] nvme: Export fast_io_fail_tmo to sysfs
    Hi Chaitanya,

    On Wed, Mar 31, 2021 at 08:34:31PM +0000, Chaitanya Kulkarni wrote:
    > > WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred. Consider using octal permissions '0644'.
    >
    > For now keep the current style.

    Okay, I thought so too. I am just wondering if a patch for changing all
    the permission sets is acceptable. I prefer the the octal permissions ;)

    > > +static ssize_t nvme_ctrl_fast_io_fail_tmo_show(struct device *dev,
    > > + struct device_attribute *attr, char *buf)
    > > +{
    > > + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
    > > +
    > > + if (ctrl->opts->fast_io_fail_tmo == -1)
    > > + return sprintf(buf, "off\n");
    > > + return sprintf(buf, "%d\n", ctrl->opts->fast_io_fail_tmo);
    >
    > do we need snprintf() for 2nd ?

    Sure thing can do. Also here I followed the existing style. The other
    store functions tend to use sprintf().

    > > + err = kstrtoint(buf, 10, &fast_io_fail_tmo);
    > > + if (err)
    > > + return -EINVAL;
    > > +
    >
    > since you are returning an error, you can remove next else if, this also
    > removes the extra line after above return. Something like this on the top
    > of yours totally untested :-

    Right, same here, followed the existing style. I prepare a patch which
    addresses your comments for the existing code as well.

    Thanks,
    Daniel

    \
     
     \ /
      Last update: 2021-04-01 09:37    [W:3.144 / U:1.392 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site