linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Logan Gunthorpe <logang@deltatee.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-nvdimm <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH 1/3] cdev: Finish the cdev api with queued mode support
Date: Wed, 20 Jan 2021 12:20:59 -0800	[thread overview]
Message-ID: <CAPcyv4jvGfZ1W8KLPO22oYVDBiUYius+Nf8kRNP=xmPvjg+deA@mail.gmail.com> (raw)
In-Reply-To: <20210120194609.GA3843758@infradead.org>

On Wed, Jan 20, 2021 at 11:46 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> The subject doesn't make any sense to me.
>
> But thn again queued sound really weird.  You just have a managed
> API with a refcount and synchronization, right?

Correct.

"queue" was in reference to the way q_usage_count behaves, but yes,
just refcount + synchronization is all this is.

>
> procfs and debugfs already support these kind of managed ops, kinda sad
> to duplicate this concept yet another time.

Oh, I didn't realize there were managed ops there, I'll go take a look
and see if cdev can adopt that scheme.

> > +static long cdev_queued_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>
> Overly long line.
>
> > +__must_check int __cdev_register_queued(struct cdev *cdev, struct module *owner,
> > +                                     dev_t dev, unsigned count,
> > +                                     const struct cdev_operations *qops)
> > +{
> > +     int rc;
> > +
> > +     if (!qops->ioctl || !owner)
> > +             return -EINVAL;
>
> Why is the ioctl method mandatory?

Yeah, that can drop. It was there more to ask the question about
whether cdev should be mandating ioctls with pointer arguments and
taking the need to specify the compat fallback away from a driver
responsibility.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

  reply	other threads:[~2021-01-20 20:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20 19:38 [PATCH 0/3] cdev: Generic shutdown handling Dan Williams
2021-01-20 19:38 ` [PATCH 1/3] cdev: Finish the cdev api with queued mode support Dan Williams
2021-01-20 19:46   ` Christoph Hellwig
2021-01-20 20:20     ` Dan Williams [this message]
2021-01-20 21:39       ` Dan Williams
2021-01-20 19:50   ` Logan Gunthorpe
2021-01-20 20:38     ` Dan Williams
2021-01-21  8:13   ` Greg KH
2021-01-21 17:50     ` Dan Williams
2021-01-20 19:39 ` [PATCH 2/3] libnvdimm/ida: Switch to non-deprecated ida helpers Dan Williams
2021-01-20 19:39 ` [PATCH 3/3] libnvdimm/ioctl: Switch to cdev_register_queued() Dan Williams
2021-01-21  8:15   ` Greg KH
2021-01-21 17:02     ` Dan Williams
2021-01-30  1:59 ` [PATCH 0/3] cdev: Generic shutdown handling Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPcyv4jvGfZ1W8KLPO22oYVDBiUYius+Nf8kRNP=xmPvjg+deA@mail.gmail.com' \
    --to=dan.j.williams@intel.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hans.verkuil@cisco.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=logang@deltatee.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).