* Re: [PATCH v4 16/24] fpga: dfl: add fpga manager platform driver for FME
From: Moritz Fischer @ 2018-03-21 16:55 UTC (permalink / raw)
To: Wu Hao
Cc: Alan Tull, Moritz Fischer, linux-fpga, linux-kernel, linux-api,
Kang, Luwei, Zhang, Yi Z, Tim Whisonant, Enno Luebbers, Shiva Rao,
Christopher Rauer, Xiao Guangrong
In-Reply-To: <20180321025001.GA3489@hao-dev>
On Wed, Mar 21, 2018 at 10:50:01AM +0800, Wu Hao wrote:
> On Tue, Mar 20, 2018 at 03:32:34PM -0500, Alan Tull wrote:
> > On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao <hao.wu@intel.com> wrote:
> >
> > Hi Hao,
> >
> > Elsewhere we discussed moving #defines used only in this driver either
> > to this .c file or to a similarly named .h file. A couple minor
> > things below.
>
> Hi Alan,
>
> Yes, I will move those #defines into a similarly named .h file.
>
> >
> > > This patch adds fpga manager driver for FPGA Management Engine (FME). It
> > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > >
> > > Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
> > > Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
> > > Signed-off-by: Shiva Rao <shiva.rao@intel.com>
> > > Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
> > > Signed-off-by: Kang Luwei <luwei.kang@intel.com>
> > > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > > Signed-off-by: Wu Hao <hao.wu@intel.com>
> > > ---
> > > v3: rename driver to dfl-fpga-fme-mgr
> > > implemented status callback for fpga manager
> > > rebased due to fpga api changes
> > > v4: rename to dfl-fme-mgr, and fix SPDX license issue
> > > add pr_credit comments and improve dev_err message
> > > remove interface_id sysfs interface
> > > include dfl-fme-pr.h instead of dfl.h
> > > ---
> > > drivers/fpga/Kconfig | 6 +
> > > drivers/fpga/Makefile | 1 +
> > > drivers/fpga/dfl-fme-mgr.c | 290 +++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 297 insertions(+)
> > > create mode 100644 drivers/fpga/dfl-fme-mgr.c
> > >
> > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > index 103d5e2..89f76e8 100644
> > > --- a/drivers/fpga/Kconfig
> > > +++ b/drivers/fpga/Kconfig
> > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> > > FPGA platform level management features. There shall be 1 FME
> > > per DFL based FPGA device.
> > >
> > > +config FPGA_DFL_FME_MGR
> > > + tristate "FPGA DFL FME Manager Driver"
> > > + depends on FPGA_DFL_FME
> > > + help
> > > + Say Y to enable FPGA Manager driver for FPGA Management Engine.
> > > +
> > > config FPGA_DFL_PCI
> > > tristate "FPGA Device Feature List (DFL) PCIe Device Driver"
> > > depends on PCI && FPGA_DFL
> > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > index 3c44fc9..f82814a 100644
> > > --- a/drivers/fpga/Makefile
> > > +++ b/drivers/fpga/Makefile
> > > @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
> > > # FPGA Device Feature List Support
> > > obj-$(CONFIG_FPGA_DFL) += dfl.o
> > > obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
> > > +obj-$(CONFIG_FPGA_DFL_FME_MGR) += dfl-fme-mgr.o
> > >
> > > dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
> > >
> > > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> > > new file mode 100644
> > > index 0000000..2f92c29
> > > --- /dev/null
> > > +++ b/drivers/fpga/dfl-fme-mgr.c
> > > @@ -0,0 +1,290 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * FPGA Manager Driver for FPGA Management Engine (FME)
> > > + *
> > > + * Copyright (C) 2017 Intel Corporation, Inc.
> > > + *
> > > + * Authors:
> > > + * Kang Luwei <luwei.kang@intel.com>
> > > + * Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > > + * Wu Hao <hao.wu@intel.com>
> > > + * Joseph Grecco <joe.grecco@intel.com>
> > > + * Enno Luebbers <enno.luebbers@intel.com>
> > > + * Tim Whisonant <tim.whisonant@intel.com>
> > > + * Ananda Ravuri <ananda.ravuri@intel.com>
> > > + * Christopher Rauer <christopher.rauer@intel.com>
> > > + * Henry Mitchel <henry.mitchel@intel.com>
> > > + */
> > > +
> > > +#include <linux/bitfield.h>
> > > +#include <linux/module.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/fpga/fpga-mgr.h>
> > > +
> > > +#include "dfl-fme-pr.h"
> > > +
> > > +#define PR_WAIT_TIMEOUT 8000000
> > > +#define PR_HOST_STATUS_IDLE 0
> > > +
> > > +struct fme_mgr_priv {
> > > + void __iomem *ioaddr;
> > > + u64 pr_error;
> > > +};
> > > +
> > > +static u64 pr_error_to_mgr_status(u64 err)
> > > +{
> > > + u64 status = 0;
> > > +
> > > + if (err & FME_PR_ERR_OPERATION_ERR)
> > > + status |= FPGA_MGR_STATUS_OPERATION_ERR;
> > > + if (err & FME_PR_ERR_CRC_ERR)
> > > + status |= FPGA_MGR_STATUS_CRC_ERR;
> > > + if (err & FME_PR_ERR_INCOMPATIBLE_BS)
> > > + status |= FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR;
> > > + if (err & FME_PR_ERR_PROTOCOL_ERR)
> > > + status |= FPGA_MGR_STATUS_IP_PROTOCOL_ERR;
> > > + if (err & FME_PR_ERR_FIFO_OVERFLOW)
> > > + status |= FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR;
> > > +
> > > + return status;
> > > +}
> > > +
> > > +static u64 fme_mgr_pr_error_handle(void __iomem *fme_pr)
> > > +{
> > > + u64 pr_status, pr_error;
> > > +
> > > + pr_status = readq(fme_pr + FME_PR_STS);
> > > + if (!(pr_status & FME_PR_STS_PR_STS))
> > > + return 0;
> > > +
> > > + pr_error = readq(fme_pr + FME_PR_ERR);
> > > + writeq(pr_error, fme_pr + FME_PR_ERR);
> > > +
> > > + return pr_error;
> > > +}
> > > +
> > > +static int fme_mgr_write_init(struct fpga_manager *mgr,
> > > + struct fpga_image_info *info,
> > > + const char *buf, size_t count)
> > > +{
> > > + struct device *dev = &mgr->dev;
> > > + struct fme_mgr_priv *priv = mgr->priv;
> > > + void __iomem *fme_pr = priv->ioaddr;
> > > + u64 pr_ctrl, pr_status;
> > > +
> > > + if (!(info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
> > > + dev_err(dev, "only supports partial reconfiguration.\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + dev_dbg(dev, "resetting PR before initiated PR\n");
> > > +
> > > + pr_ctrl = readq(fme_pr + FME_PR_CTRL);
> > > + pr_ctrl |= FME_PR_CTRL_PR_RST;
> > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL);
> > > +
> > > + if (readq_poll_timeout(fme_pr + FME_PR_CTRL, pr_ctrl,
> > > + pr_ctrl & FME_PR_CTRL_PR_RSTACK, 1,
> > > + PR_WAIT_TIMEOUT)) {
> > > + dev_err(dev, "PR Reset ACK timeout\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + pr_ctrl = readq(fme_pr + FME_PR_CTRL);
> > > + pr_ctrl &= ~FME_PR_CTRL_PR_RST;
> > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL);
> > > +
> > > + dev_dbg(dev,
> > > + "waiting for PR resource in HW to be initialized and ready\n");
> > > +
> > > + if (readq_poll_timeout(fme_pr + FME_PR_STS, pr_status,
> > > + (pr_status & FME_PR_STS_PR_STS) ==
> > > + FME_PR_STS_PR_STS_IDLE, 1, PR_WAIT_TIMEOUT)) {
> > > + dev_err(dev, "PR Status timeout\n");
> > > + priv->pr_error = fme_mgr_pr_error_handle(fme_pr);
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + dev_dbg(dev, "check and clear previous PR error\n");
> > > + priv->pr_error = fme_mgr_pr_error_handle(fme_pr);
> > > + if (priv->pr_error)
> > > + dev_dbg(dev, "previous PR error detected %llx\n",
> > > + (unsigned long long)priv->pr_error);
> > > +
> > > + dev_dbg(dev, "set PR port ID\n");
> > > +
> > > + pr_ctrl = readq(fme_pr + FME_PR_CTRL);
> > > + pr_ctrl &= ~FME_PR_CTRL_PR_RGN_ID;
> > > + pr_ctrl |= FIELD_PREP(FME_PR_CTRL_PR_RGN_ID, info->region_id);
> > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int fme_mgr_write(struct fpga_manager *mgr,
> > > + const char *buf, size_t count)
> > > +{
> > > + struct device *dev = &mgr->dev;
> > > + struct fme_mgr_priv *priv = mgr->priv;
> > > + void __iomem *fme_pr = priv->ioaddr;
> > > + u64 pr_ctrl, pr_status, pr_data;
> > > + int delay = 0, pr_credit, i = 0;
> > > +
> > > + dev_dbg(dev, "start request\n");
> > > +
> > > + pr_ctrl = readq(fme_pr + FME_PR_CTRL);
> > > + pr_ctrl |= FME_PR_CTRL_PR_START;
> > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL);
> > > +
> > > + dev_dbg(dev, "pushing data from bitstream to HW\n");
> > > +
> > > + /*
> > > + * driver can push data to PR hardware using PR_DATA register once HW
> > > + * has enough pr_credit (> 1), pr_credit reduces one for every 32bit
> > > + * pr data write to PR_DATA register. If pr_credit <= 1, driver needs
> > > + * to wait for enough pr_credit from hardware by polling.
> > > + */
> > > + pr_status = readq(fme_pr + FME_PR_STS);
> > > + pr_credit = FIELD_GET(FME_PR_STS_PR_CREDIT, pr_status);
> > > +
> > > + while (count > 0) {
> > > + while (pr_credit <= 1) {
> > > + if (delay++ > PR_WAIT_TIMEOUT) {
> > > + dev_err(dev, "PR_CREDIT timeout\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > + udelay(1);
> > > +
> > > + pr_status = readq(fme_pr + FME_PR_STS);
> > > + pr_credit = FIELD_GET(FME_PR_STS_PR_CREDIT, pr_status);
> > > + }
> > > +
> > > + if (count >= 4) {
> > > + pr_data = 0;
> > > + pr_data |= FIELD_PREP(FME_PR_DATA_PR_DATA_RAW,
> > > + *(((u32 *)buf) + i));
> > > + writeq(pr_data, fme_pr + FME_PR_DATA);
> > > + count -= 4;
> > > + pr_credit--;
> > > + i++;
> > > + } else {
> > > + WARN_ON(1);
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int fme_mgr_write_complete(struct fpga_manager *mgr,
> > > + struct fpga_image_info *info)
> > > +{
> > > + struct device *dev = &mgr->dev;
> > > + struct fme_mgr_priv *priv = mgr->priv;
> > > + void __iomem *fme_pr = priv->ioaddr;
> > > + u64 pr_ctrl;
> > > +
> > > + pr_ctrl = readq(fme_pr + FME_PR_CTRL);
> > > + pr_ctrl |= FME_PR_CTRL_PR_COMPLETE;
> > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL);
> > > +
> > > + dev_dbg(dev, "green bitstream push complete\n");
> > > + dev_dbg(dev, "waiting for HW to release PR resource\n");
> > > +
> > > + if (readq_poll_timeout(fme_pr + FME_PR_CTRL, pr_ctrl,
> > > + !(pr_ctrl & FME_PR_CTRL_PR_START), 1,
> > > + PR_WAIT_TIMEOUT)) {
> > > + dev_err(dev, "PR Completion ACK timeout.\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + dev_dbg(dev, "PR operation complete, checking status\n");
> > > + priv->pr_error = fme_mgr_pr_error_handle(fme_pr);
> > > + if (priv->pr_error) {
> > > + dev_dbg(dev, "PR error detected %llx\n",
> > > + (unsigned long long)priv->pr_error);
> > > + return -EIO;
> > > + }
> > > +
> > > + dev_dbg(dev, "PR done successfully\n");
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static enum fpga_mgr_states fme_mgr_state(struct fpga_manager *mgr)
> > > +{
> > > + return FPGA_MGR_STATE_UNKNOWN;
> > > +}
> > > +
> > > +static u64 fme_mgr_status(struct fpga_manager *mgr)
> > > +{
> > > + struct fme_mgr_priv *priv = mgr->priv;
> > > +
> > > + return pr_error_to_mgr_status(priv->pr_error);
> > > +}
> > > +
> > > +static const struct fpga_manager_ops fme_mgr_ops = {
> > > + .write_init = fme_mgr_write_init,
> > > + .write = fme_mgr_write,
> > > + .write_complete = fme_mgr_write_complete,
> > > + .state = fme_mgr_state,
> > > + .status = fme_mgr_status,
> > > +};
> > > +
> > > +static int fme_mgr_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct fme_mgr_priv *priv;
> > > + struct fpga_manager *mgr;
> > > + struct resource *res;
> > > + int ret;
> > > +
> > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + priv->ioaddr = devm_ioremap(dev, res->start, resource_size(res));
> >
> > How about using devm_ioremap_resourc(dev, res) here instead?
>
> Actually the register region has already been mapped in lower level driver
> (e.g pci) so I think we don't have to map the second time here. I plan to
> add some code to pass the ioaddr via the platform data, and check if valid
> ioaddr from the platform data firstly in this probe function. If no pdata
> or no valid ioaddr, then go with devm_ioremap_resource. :)
If you end up sharing register spaces between drivers is regmap / syscon
maybe a good idea?
>
> >
> > > + if (IS_ERR(priv->ioaddr))
> > > + return PTR_ERR(priv->ioaddr);
> > > +
> > > + mgr = devm_kzalloc(dev, sizeof(*mgr), GFP_KERNEL);
> > > + if (!mgr)
> > > + return -ENOMEM;
> > > +
> > > + mgr->name = "DFL FPGA Manager";
> > > + mgr->mops = &fme_mgr_ops;
> > > + mgr->priv = priv;
> > > + mgr->parent = dev;
> > > + platform_set_drvdata(pdev, mgr);
> > > +
> > > + ret = fpga_mgr_register(mgr);
> > > + if (ret)
> > > + dev_err(dev, "unable to register FPGA manager\n");
> > > +
> > > + return ret;
> >
> > You can probably just do "return fpga_mgr_register(mgr);" here.
>
> Yes, it looks better, I will fix it. Thanks a lot for the review.
>
> Hao
>
> >
> > Thanks,
> > Alan
> >
> > > +}
> > > +
> > > +static int fme_mgr_remove(struct platform_device *pdev)
> > > +{
> > > + struct fpga_manager *mgr = platform_get_drvdata(pdev);
> > > +
> > > + fpga_mgr_unregister(mgr);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct platform_driver fme_mgr_driver = {
> > > + .driver = {
> > > + .name = FPGA_DFL_FME_MGR,
> > > + },
> > > + .probe = fme_mgr_probe,
> > > + .remove = fme_mgr_remove,
> > > +};
> > > +
> > > +module_platform_driver(fme_mgr_driver);
> > > +
> > > +MODULE_DESCRIPTION("FPGA Manager for DFL FPGA Management Engine");
> > > +MODULE_AUTHOR("Intel Corporation");
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_ALIAS("platform:dfl-fme-mgr");
> > > --
> > > 2.7.4
> > >
Cheers,
Moritz
^ permalink raw reply
* Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL
From: Darrick J. Wong @ 2018-03-21 16:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-7-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:10AM +0100, Christoph Hellwig wrote:
> Simple one-shot poll through the io_submit() interface. To poll for
> a file descriptor the application should submit an iocb of type
> IOCB_CMD_POLL. It will poll the fd for the events specified in the
> the first 32 bits of the aio_buf field of the iocb.
>
> Unlike poll or epoll without EPOLLONESHOT this interface always works
> in one shot mode, that is once the iocb is completed, it will have to be
> resubmitted.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
After this point in the series my familiarity with network code and
sockets drops way off, so please don't be too surprised if I don't get
any further.
--D
> ---
> fs/aio.c | 102 ++++++++++++++++++++++++++++++++++++++++++-
> include/uapi/linux/aio_abi.h | 6 +--
> 2 files changed, 103 insertions(+), 5 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 79d3eb3d2dd9..38b408129697 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -5,6 +5,7 @@
> * Implements an efficient asynchronous io interface.
> *
> * Copyright 2000, 2001, 2002 Red Hat, Inc. All Rights Reserved.
> + * Copyright 2018 Christoph Hellwig.
> *
> * See ../COPYING for licensing terms.
> */
> @@ -162,10 +163,18 @@ struct fsync_iocb {
> bool datasync;
> };
>
> +struct poll_iocb {
> + struct file *file;
> + __poll_t events;
> + struct wait_queue_head *head;
> + struct wait_queue_entry wait;
> +};
> +
> struct aio_kiocb {
> union {
> struct kiocb rw;
> struct fsync_iocb fsync;
> + struct poll_iocb poll;
> };
>
> struct kioctx *ki_ctx;
> @@ -1590,7 +1599,6 @@ static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
> return -EINVAL;
> if (iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)
> return -EINVAL;
> -
> req->file = fget(iocb->aio_fildes);
> if (unlikely(!req->file))
> return -EBADF;
> @@ -1609,6 +1617,96 @@ static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
> return ret;
> }
>
> +static void __aio_complete_poll(struct poll_iocb *req, __poll_t mask)
> +{
> + fput(req->file);
> + aio_complete(container_of(req, struct aio_kiocb, poll),
> + mangle_poll(mask), 0);
> +}
> +
> +static void aio_complete_poll(struct poll_iocb *req, __poll_t mask)
> +{
> + struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, poll);
> +
> + if (!(iocb->flags & AIO_IOCB_CANCELLED))
> + __aio_complete_poll(req, mask);
> +}
> +
> +static int aio_poll_cancel(struct kiocb *rw)
> +{
> + struct aio_kiocb *iocb = container_of(rw, struct aio_kiocb, rw);
> +
> + remove_wait_queue(iocb->poll.head, &iocb->poll.wait);
> + __aio_complete_poll(&iocb->poll, 0); /* no events to report */
> + return 0;
> +}
> +
> +static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
> + void *key)
> +{
> + struct poll_iocb *req = container_of(wait, struct poll_iocb, wait);
> + struct file *file = req->file;
> + __poll_t mask = key_to_poll(key);
> +
> + assert_spin_locked(&req->head->lock);
> +
> + /* for instances that support it check for an event match first: */
> + if (mask && !(mask & req->events))
> + return 0;
> +
> + mask = vfs_poll_mask(file, req->events);
> + if (!mask)
> + return 0;
> +
> + __remove_wait_queue(req->head, &req->wait);
> + aio_complete_poll(req, mask);
> + return 1;
> +}
> +
> +static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb)
> +{
> + struct poll_iocb *req = &aiocb->poll;
> + unsigned long flags;
> + __poll_t mask;
> +
> + /* reject any unknown events outside the normal event mask. */
> + if ((u16)iocb->aio_buf != iocb->aio_buf)
> + return -EINVAL;
> + /* reject fields that are not defined for poll */
> + if (iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)
> + return -EINVAL;
> +
> + req->events = demangle_poll(iocb->aio_buf) | POLLERR | POLLHUP;
> + req->file = fget(iocb->aio_fildes);
> + if (unlikely(!req->file))
> + return -EBADF;
> +
> + req->head = vfs_get_poll_head(req->file, req->events);
> + if (!req->head) {
> + fput(req->file);
> + return -EINVAL; /* same as no support for IOCB_CMD_POLL */
> + }
> + if (IS_ERR(req->head)) {
> + mask = PTR_TO_POLL(req->head);
> + goto done;
> + }
> +
> + init_waitqueue_func_entry(&req->wait, aio_poll_wake);
> +
> + spin_lock_irqsave(&req->head->lock, flags);
> + mask = vfs_poll_mask(req->file, req->events);
> + if (!mask) {
> + __kiocb_set_cancel_fn(aiocb, aio_poll_cancel,
> + AIO_IOCB_DELAYED_CANCEL);
> + __add_wait_queue(req->head, &req->wait);
> + }
> + spin_unlock_irqrestore(&req->head->lock, flags);
> +done:
> + if (mask)
> + aio_complete_poll(req, mask);
> + return -EIOCBQUEUED;
> +}
> +
> static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> struct iocb *iocb, bool compat)
> {
> @@ -1677,6 +1775,8 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> break;
> case IOCB_CMD_FDSYNC:
> ret = aio_fsync(&req->fsync, iocb, true);
> + case IOCB_CMD_POLL:
> + ret = aio_poll(req, iocb);
> break;
> default:
> pr_debug("invalid aio operation %d\n", iocb->aio_lio_opcode);
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index 2c0a3415beee..ed0185945bb2 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -39,10 +39,8 @@ enum {
> IOCB_CMD_PWRITE = 1,
> IOCB_CMD_FSYNC = 2,
> IOCB_CMD_FDSYNC = 3,
> - /* These two are experimental.
> - * IOCB_CMD_PREADX = 4,
> - * IOCB_CMD_POLL = 5,
> - */
> + /* 4 was the experimental IOCB_CMD_PREADX */
> + IOCB_CMD_POLL = 5,
> IOCB_CMD_NOOP = 6,
> IOCB_CMD_PREADV = 7,
> IOCB_CMD_PWRITEV = 8,
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/28] fs: introduce new ->get_poll_head and ->poll_mask methods
From: Darrick J. Wong @ 2018-03-21 16:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-6-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:09AM +0100, Christoph Hellwig wrote:
> ->get_poll_head returns the waitqueue that the poll operation is going
> to sleep on. Note that this means we can only use a single waitqueue
> for the poll, unlike some current drivers that use two waitqueues for
> different events. But now that we have keyed wakeups and heavily use
> those for poll there aren't that many good reason left to keep the
> multiple waitqueues, and if there are any ->poll is still around, the
> driver just won't support aio poll.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> Documentation/filesystems/Locking | 7 ++++++-
> Documentation/filesystems/vfs.txt | 13 +++++++++++++
> fs/select.c | 28 ++++++++++++++++++++++++++++
> include/linux/fs.h | 2 ++
> include/linux/poll.h | 27 +++++++++++++++++++++++----
> 5 files changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
> index 220bba28f72b..6d227f9d7bd9 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -440,6 +440,8 @@ prototypes:
> ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
> int (*iterate) (struct file *, struct dir_context *);
> __poll_t (*poll) (struct file *, struct poll_table_struct *);
> + struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
> + __poll_t (*poll_mask) (struct file *, __poll_t);
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> @@ -470,7 +472,7 @@ prototypes:
> };
>
> locking rules:
> - All may block.
> + All except for ->poll_mask may block.
>
> ->llseek() locking has moved from llseek to the individual llseek
> implementations. If your fs is not using generic_file_llseek, you
> @@ -498,6 +500,9 @@ in sys_read() and friends.
> the lease within the individual filesystem to record the result of the
> operation
>
> +->poll_mask can be called with or without the waitqueue lock for the waitqueue
> +returned from ->get_poll_head.
> +
> --------------------------- dquot_operations -------------------------------
> prototypes:
> int (*write_dquot) (struct dquot *);
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index f608180ad59d..50ee13563271 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -857,6 +857,8 @@ struct file_operations {
> ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
> int (*iterate) (struct file *, struct dir_context *);
> __poll_t (*poll) (struct file *, struct poll_table_struct *);
> + struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
> + __poll_t (*poll_mask) (struct file *, __poll_t);
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> @@ -901,6 +903,17 @@ otherwise noted.
> activity on this file and (optionally) go to sleep until there
> is activity. Called by the select(2) and poll(2) system calls
>
> + get_poll_head: Returns the struct wait_queue_head that poll, select,
> + epoll or aio poll should wait on in case this instance only has single
> + waitqueue. Can return NULL to indicate polling is not supported,
> + or a POLL* value using the POLL_TO_PTR helper in case a grave error
> + occured and ->poll_mask shall not be called.
> +
> + poll_mask: return the mask of POLL* values describing the file descriptor
> + state. Called either before going to sleep on the waitqueue returned by
> + get_poll_head, or after it has been woken. If ->get_poll_head and
> + ->poll_mask are implemented ->poll does not need to be implement.
> +
> unlocked_ioctl: called by the ioctl(2) system call.
>
> compat_ioctl: called by the ioctl(2) system call when 32 bit system calls
> diff --git a/fs/select.c b/fs/select.c
> index ba91103707ea..cc270d7f6192 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -34,6 +34,34 @@
>
> #include <linux/uaccess.h>
>
> +__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
> +{
> + unsigned int events = poll_requested_events(pt);
> + struct wait_queue_head *head;
> +
> + if (unlikely(!file_can_poll(file)))
> + return DEFAULT_POLLMASK;
> +
> + if (file->f_op->poll)
> + return file->f_op->poll(file, pt);
> +
> + /*
> + * Only get the poll head and do the first mask check if we are actually
> + * going to sleep on this file:
> + */
> + if (pt && pt->_qproc) {
> + head = vfs_get_poll_head(file, events);
> + if (!head)
> + return DEFAULT_POLLMASK;
> + if (IS_ERR(head))
> + return PTR_TO_POLL(head);
> +
> + pt->_qproc(file, head, pt);
> + }
> +
> + return file->f_op->poll_mask(file, events);
> +}
> +EXPORT_SYMBOL_GPL(vfs_poll);
>
> /*
> * Estimate expected accuracy in ns from a timeval.
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 79c413985305..6ea2c0843bb1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1708,6 +1708,8 @@ struct file_operations {
> int (*iterate) (struct file *, struct dir_context *);
> int (*iterate_shared) (struct file *, struct dir_context *);
> __poll_t (*poll) (struct file *, struct poll_table_struct *);
> + struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
> + __poll_t (*poll_mask) (struct file *, __poll_t);
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> diff --git a/include/linux/poll.h b/include/linux/poll.h
> index 7e0fdcf905d2..42e8e8665fb0 100644
> --- a/include/linux/poll.h
> +++ b/include/linux/poll.h
> @@ -74,18 +74,37 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
> pt->_key = ~(__poll_t)0; /* all events enabled */
> }
>
> +/*
> + * ->get_poll_head can return a __poll_t in the PTR_ERR, use these macros
> + * to return the value and recover it. It takes care of the negation as
> + * well as off the annotations.
> + */
> +#define POLL_TO_PTR(mask) (ERR_PTR(-(__force int)(mask)))
> +#define PTR_TO_POLL(ptr) ((__force __poll_t)-PTR_ERR((ptr)))
> +
> static inline bool file_can_poll(struct file *file)
> {
> - return file->f_op->poll;
> + return file->f_op->poll ||
> + (file->f_op->get_poll_head && file->f_op->poll_mask);
> }
>
> -static inline __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
> +static inline struct wait_queue_head *vfs_get_poll_head(struct file *file,
> + __poll_t events)
> {
> - if (unlikely(!file->f_op->poll))
> + if (unlikely(!file->f_op->get_poll_head || !file->f_op->poll_mask))
> + return NULL;
> + return file->f_op->get_poll_head(file, events);
> +}
> +
> +static inline __poll_t vfs_poll_mask(struct file *file, __poll_t events)
> +{
> + if (unlikely(!file->f_op->poll_mask))
> return DEFAULT_POLLMASK;
> - return file->f_op->poll(file, pt);
> + return file->f_op->poll_mask(file, events) & events;
> }
>
> +__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt);
> +
> struct poll_table_entry {
> struct file *filp;
> __poll_t key;
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 03/28] fs: update documentation to mention __poll_t
From: Darrick J. Wong @ 2018-03-21 16:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-4-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:07AM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
It still would be nice to have a commit message to note that we're
merely updating the documentation to reflect what's currently in the C
headers (as opposed to a mangled patch that updated the docs but dropped
the actual code update)...
...so with that fixed,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> Documentation/filesystems/Locking | 2 +-
> Documentation/filesystems/vfs.txt | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
> index 75d2d57e2c44..220bba28f72b 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -439,7 +439,7 @@ prototypes:
> ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
> ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
> int (*iterate) (struct file *, struct dir_context *);
> - unsigned int (*poll) (struct file *, struct poll_table_struct *);
> + __poll_t (*poll) (struct file *, struct poll_table_struct *);
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index 5fd325df59e2..f608180ad59d 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -856,7 +856,7 @@ struct file_operations {
> ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
> ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
> int (*iterate) (struct file *, struct dir_context *);
> - unsigned int (*poll) (struct file *, struct poll_table_struct *);
> + __poll_t (*poll) (struct file *, struct poll_table_struct *);
> long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> int (*mmap) (struct file *, struct vm_area_struct *);
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 8/9] aio: implement io_pgetevents
From: Darrick J. Wong @ 2018-03-21 16:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-9-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:31AM +0100, Christoph Hellwig wrote:
> This is the io_getevents equivalent of ppoll/pselect and allows to
> properly mix signals and aio completions (especially with IOCB_CMD_POLL)
> and atomically executes the following sequence:
>
> sigset_t origmask;
>
> pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
> ret = io_getevents(ctx, min_nr, nr, events, timeout);
> pthread_sigmask(SIG_SETMASK, &origmask, NULL);
>
> Note that unlike many other signal related calls we do not pass a sigmask
> size, as that would get us to 7 arguments, which aren't easily supported
> by the syscall infrastructure. It seems a lot less painful to just add a
> new syscall variant in the unlikely case we're going to increase the
> sigset size.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> fs/aio.c | 114 ++++++++++++++++++++++++++++++---
> include/linux/compat.h | 7 ++
> include/linux/syscalls.h | 6 ++
> include/uapi/asm-generic/unistd.h | 4 +-
> include/uapi/linux/aio_abi.h | 6 ++
> kernel/sys_ni.c | 2 +
> 8 files changed, 130 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 2a5e99cff859..c1018580ddaa 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -391,3 +391,4 @@
> 382 i386 pkey_free sys_pkey_free
> 383 i386 statx sys_statx
> 384 i386 arch_prctl sys_arch_prctl compat_sys_arch_prctl
> +385 i386 io_pgetevents sys_io_pgetevents compat_sys_io_pgetevents
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 5aef183e2f85..e995cd2b4e65 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -339,6 +339,7 @@
> 330 common pkey_alloc sys_pkey_alloc
> 331 common pkey_free sys_pkey_free
> 332 common statx sys_statx
> +333 common io_pgetevents sys_io_pgetevents
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/aio.c b/fs/aio.c
> index 9d7d6e4cde87..da87cbf7c67a 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1291,10 +1291,6 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
> wait_event_interruptible_hrtimeout(ctx->wait,
> aio_read_events(ctx, min_nr, nr, event, &ret),
> until);
> -
> - if (!ret && signal_pending(current))
> - ret = -EINTR;
> -
> return ret;
> }
>
> @@ -1874,13 +1870,60 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
> struct timespec __user *, timeout)
> {
> struct timespec64 ts;
> + int ret;
> +
> + if (timeout && unlikely(get_timespec64(&ts, timeout)))
> + return -EFAULT;
> +
> + ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
> + if (!ret && signal_pending(current))
> + ret = -EINTR;
> + return ret;
> +}
> +
> +SYSCALL_DEFINE6(io_pgetevents,
> + aio_context_t, ctx_id,
> + long, min_nr,
> + long, nr,
> + struct io_event __user *, events,
> + struct timespec __user *, timeout,
> + const struct __aio_sigset __user *, usig)
> +{
> + struct __aio_sigset ksig = { NULL, };
> + sigset_t ksigmask, sigsaved;
> + struct timespec64 ts;
> + int ret;
> +
> + if (timeout && unlikely(get_timespec64(&ts, timeout)))
> + return -EFAULT;
>
> - if (timeout) {
> - if (unlikely(get_timespec64(&ts, timeout)))
> + if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
> + return -EFAULT;
> +
> + if (ksig.sigmask) {
> + if (ksig.sigsetsize != sizeof(sigset_t))
> + return -EINVAL;
> + if (copy_from_user(&ksigmask, ksig.sigmask, sizeof(ksigmask)))
> return -EFAULT;
> + sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
> + sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
> + }
> +
> + ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
> + if (signal_pending(current)) {
> + if (ksig.sigmask) {
> + current->saved_sigmask = sigsaved;
> + set_restore_sigmask();
> + }
> +
> + if (!ret)
> + ret = -ERESTARTNOHAND;
> + } else {
> + if (ksig.sigmask)
> + sigprocmask(SIG_SETMASK, &sigsaved, NULL);
> }
>
> - return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
> + return ret;
> }
>
> #ifdef CONFIG_COMPAT
> @@ -1891,13 +1934,64 @@ COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
> struct compat_timespec __user *, timeout)
> {
> struct timespec64 t;
> + int ret;
> +
> + if (timeout && compat_get_timespec64(&t, timeout))
> + return -EFAULT;
> +
> + ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
> + if (!ret && signal_pending(current))
> + ret = -EINTR;
> + return ret;
> +}
> +
> +
> +struct __compat_aio_sigset {
> + compat_sigset_t __user *sigmask;
> + compat_size_t sigsetsize;
> +};
> +
> +COMPAT_SYSCALL_DEFINE6(io_pgetevents,
> + compat_aio_context_t, ctx_id,
> + compat_long_t, min_nr,
> + compat_long_t, nr,
> + struct io_event __user *, events,
> + struct compat_timespec __user *, timeout,
> + const struct __compat_aio_sigset __user *, usig)
> +{
> + struct __compat_aio_sigset ksig = { NULL, };
> + sigset_t ksigmask, sigsaved;
> + struct timespec64 t;
> + int ret;
> +
> + if (timeout && compat_get_timespec64(&t, timeout))
> + return -EFAULT;
>
> - if (timeout) {
> - if (compat_get_timespec64(&t, timeout))
> + if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
> + return -EFAULT;
> +
> + if (ksig.sigmask) {
> + if (ksig.sigsetsize != sizeof(compat_sigset_t))
> + return -EINVAL;
> + if (get_compat_sigset(&ksigmask, ksig.sigmask))
> return -EFAULT;
> + sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
> + sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
> + }
>
> + ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
> + if (signal_pending(current)) {
> + if (ksig.sigmask) {
> + current->saved_sigmask = sigsaved;
> + set_restore_sigmask();
> + }
> + if (!ret)
> + ret = -ERESTARTNOHAND;
> + } else {
> + if (ksig.sigmask)
> + sigprocmask(SIG_SETMASK, &sigsaved, NULL);
> }
>
> - return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
> + return ret;
> }
> #endif
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 16c3027074a2..d80cda61eafd 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -304,6 +304,7 @@ extern int put_compat_rusage(const struct rusage *,
> struct compat_rusage __user *);
>
> struct compat_siginfo;
> +struct __compat_aio_sigset;
>
> extern asmlinkage long compat_sys_waitid(int, compat_pid_t,
> struct compat_siginfo __user *, int,
> @@ -656,6 +657,12 @@ asmlinkage long compat_sys_io_getevents(compat_aio_context_t ctx_id,
> compat_long_t nr,
> struct io_event __user *events,
> struct compat_timespec __user *timeout);
> +asmlinkage long compat_sys_io_pgetevents(compat_aio_context_t ctx_id,
> + compat_long_t min_nr,
> + compat_long_t nr,
> + struct io_event __user *events,
> + struct compat_timespec __user *timeout,
> + const struct __compat_aio_sigset __user *usig);
> asmlinkage long compat_sys_io_submit(compat_aio_context_t ctx_id, int nr,
> u32 __user *iocb);
> asmlinkage long compat_sys_mount(const char __user *dev_name,
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index a78186d826d7..8515ec53c81b 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -539,6 +539,12 @@ asmlinkage long sys_io_getevents(aio_context_t ctx_id,
> long nr,
> struct io_event __user *events,
> struct timespec __user *timeout);
> +asmlinkage long sys_io_pgetevents(aio_context_t ctx_id,
> + long min_nr,
> + long nr,
> + struct io_event __user *events,
> + struct timespec __user *timeout,
> + const struct __aio_sigset *sig);
> asmlinkage long sys_io_submit(aio_context_t, long,
> struct iocb __user * __user *);
> asmlinkage long sys_io_cancel(aio_context_t ctx_id, struct iocb __user *iocb,
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 8b87de067bc7..ce2ebbeece10 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -732,9 +732,11 @@ __SYSCALL(__NR_pkey_alloc, sys_pkey_alloc)
> __SYSCALL(__NR_pkey_free, sys_pkey_free)
> #define __NR_statx 291
> __SYSCALL(__NR_statx, sys_statx)
> +#define __NR_io_pgetevents 292
> +__SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 292
> +#define __NR_syscalls 293
>
> /*
> * All syscalls below here should go away really,
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index a04adbc70ddf..2c0a3415beee 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -29,6 +29,7 @@
>
> #include <linux/types.h>
> #include <linux/fs.h>
> +#include <linux/signal.h>
> #include <asm/byteorder.h>
>
> typedef __kernel_ulong_t aio_context_t;
> @@ -108,5 +109,10 @@ struct iocb {
> #undef IFBIG
> #undef IFLITTLE
>
> +struct __aio_sigset {
> + sigset_t __user *sigmask;
> + size_t sigsetsize;
> +};
> +
> #endif /* __LINUX__AIO_ABI_H */
>
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index b5189762d275..8f7705559b38 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -151,9 +151,11 @@ cond_syscall(sys_io_destroy);
> cond_syscall(sys_io_submit);
> cond_syscall(sys_io_cancel);
> cond_syscall(sys_io_getevents);
> +cond_syscall(sys_io_pgetevents);
> cond_syscall(compat_sys_io_setup);
> cond_syscall(compat_sys_io_submit);
> cond_syscall(compat_sys_io_getevents);
> +cond_syscall(compat_sys_io_pgetevents);
> cond_syscall(sys_sysfs);
> cond_syscall(sys_syslog);
> cond_syscall(sys_process_vm_readv);
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 9/9] aio: implement IOCB_CMD_FSYNC and IOCB_CMD_FDSYNC
From: Darrick J. Wong @ 2018-03-21 16:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-10-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:32AM +0100, Christoph Hellwig wrote:
> Simple workqueue offload for now, but prepared for adding a real aio_fsync
> method if the need arises. Based on an earlier patch from Dave Chinner.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/aio.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index da87cbf7c67a..79d3eb3d2dd9 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -156,9 +156,16 @@ struct kioctx {
> unsigned id;
> };
>
> +struct fsync_iocb {
> + struct work_struct work;
> + struct file *file;
> + bool datasync;
> +};
> +
> struct aio_kiocb {
> union {
> struct kiocb rw;
> + struct fsync_iocb fsync;
> };
>
> struct kioctx *ki_ctx;
> @@ -1565,6 +1572,43 @@ static ssize_t aio_write(struct kiocb *req, struct iocb *iocb, bool vectored,
> return ret;
> }
>
> +static void aio_fsync_work(struct work_struct *work)
> +{
> + struct fsync_iocb *req = container_of(work, struct fsync_iocb, work);
> + int ret;
> +
> + ret = vfs_fsync(req->file, req->datasync);
> + fput(req->file);
> + aio_complete(container_of(req, struct aio_kiocb, fsync), ret, 0);
> +}
> +
> +static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
> +{
> + int ret;
> +
> + if (iocb->aio_buf)
> + return -EINVAL;
> + if (iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)
> + return -EINVAL;
> +
> + req->file = fget(iocb->aio_fildes);
> + if (unlikely(!req->file))
> + return -EBADF;
> +
> + ret = -EINVAL;
> + if (!req->file->f_op->fsync)
> + goto out_fput;
> +
> + req->datasync = datasync;
> + INIT_WORK(&req->work, aio_fsync_work);
> + schedule_work(&req->work);
> + return -EIOCBQUEUED;
> +out_fput:
> + if (unlikely(ret && ret != -EIOCBQUEUED))
> + fput(req->file);
> + return ret;
> +}
> +
> static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> struct iocb *iocb, bool compat)
> {
> @@ -1628,6 +1672,12 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> case IOCB_CMD_PWRITEV:
> ret = aio_write(&req->rw, iocb, true, compat);
> break;
> + case IOCB_CMD_FSYNC:
> + ret = aio_fsync(&req->fsync, iocb, false);
> + break;
> + case IOCB_CMD_FDSYNC:
> + ret = aio_fsync(&req->fsync, iocb, true);
> + break;
> default:
> pr_debug("invalid aio operation %d\n", iocb->aio_lio_opcode);
> ret = -EINVAL;
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 7/9] aio: add delayed cancel support
From: Darrick J. Wong @ 2018-03-21 16:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-8-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:30AM +0100, Christoph Hellwig wrote:
> The upcoming aio poll support would like to be able to complete the
> iocb inline from the cancellation context, but that would cause
> a lock order reversal. Add support for optionally moving the cancelation
> outside the context lock to avoid this reversal.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/aio.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 0b6394b4e528..9d7d6e4cde87 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -170,6 +170,10 @@ struct aio_kiocb {
> struct list_head ki_list; /* the aio core uses this
> * for cancellation */
>
> + unsigned int flags; /* protected by ctx->ctx_lock */
> +#define AIO_IOCB_DELAYED_CANCEL (1 << 0)
> +#define AIO_IOCB_CANCELLED (1 << 1)
> +
> /*
> * If the aio_resfd field of the userspace iocb is not zero,
> * this is the underlying eventfd context to deliver events to.
> @@ -536,9 +540,9 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events)
> #define AIO_EVENTS_FIRST_PAGE ((PAGE_SIZE - sizeof(struct aio_ring)) / sizeof(struct io_event))
> #define AIO_EVENTS_OFFSET (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
>
> -void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> +static void __kiocb_set_cancel_fn(struct aio_kiocb *req,
> + kiocb_cancel_fn *cancel, unsigned int iocb_flags)
> {
> - struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
> struct kioctx *ctx = req->ki_ctx;
> unsigned long flags;
>
> @@ -548,8 +552,15 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> spin_lock_irqsave(&ctx->ctx_lock, flags);
> list_add_tail(&req->ki_list, &ctx->active_reqs);
> req->ki_cancel = cancel;
> + req->flags |= iocb_flags;
> spin_unlock_irqrestore(&ctx->ctx_lock, flags);
> }
> +
> +void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> +{
> + return __kiocb_set_cancel_fn(container_of(iocb, struct aio_kiocb, rw),
> + cancel, 0);
> +}
> EXPORT_SYMBOL(kiocb_set_cancel_fn);
>
> /*
> @@ -603,17 +614,27 @@ static void free_ioctx_users(struct percpu_ref *ref)
> {
> struct kioctx *ctx = container_of(ref, struct kioctx, users);
> struct aio_kiocb *req;
> + LIST_HEAD(list);
>
> spin_lock_irq(&ctx->ctx_lock);
> -
> while (!list_empty(&ctx->active_reqs)) {
> req = list_first_entry(&ctx->active_reqs,
> struct aio_kiocb, ki_list);
> - kiocb_cancel(req);
> - }
>
> + if (req->flags & AIO_IOCB_DELAYED_CANCEL) {
> + req->flags |= AIO_IOCB_CANCELLED;
> + list_move_tail(&req->ki_list, &list);
> + } else {
> + kiocb_cancel(req);
> + }
> + }
> spin_unlock_irq(&ctx->ctx_lock);
>
> + while (!list_empty(&list)) {
> + req = list_first_entry(&list, struct aio_kiocb, ki_list);
> + kiocb_cancel(req);
> + }
> +
> percpu_ref_kill(&ctx->reqs);
> percpu_ref_put(&ctx->reqs);
> }
> @@ -1785,15 +1806,22 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
> if (unlikely(!ctx))
> return -EINVAL;
>
> - spin_lock_irq(&ctx->ctx_lock);
> + ret = -EINVAL;
>
> + spin_lock_irq(&ctx->ctx_lock);
> kiocb = lookup_kiocb(ctx, iocb, key);
> + if (kiocb) {
> + if (kiocb->flags & AIO_IOCB_DELAYED_CANCEL) {
> + kiocb->flags |= AIO_IOCB_CANCELLED;
> + } else {
> + ret = kiocb_cancel(kiocb);
> + kiocb = NULL;
> + }
> + }
> + spin_unlock_irq(&ctx->ctx_lock);
> +
> if (kiocb)
> ret = kiocb_cancel(kiocb);
> - else
> - ret = -EINVAL;
> -
> - spin_unlock_irq(&ctx->ctx_lock);
>
> if (!ret) {
> /*
> @@ -1805,7 +1833,6 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
> }
>
> percpu_ref_put(&ctx->users);
> -
> return ret;
> }
>
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 6/9] aio: delete iocbs from the active_reqs list in kiocb_cancel
From: Darrick J. Wong @ 2018-03-21 16:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-7-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:29AM +0100, Christoph Hellwig wrote:
> One we cancel an iocb there is no reason to keep it on the active_reqs
> list, given that the list is only used to look for cancelation candidates.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/aio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 2d40cf5dd4ec..0b6394b4e528 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -561,6 +561,8 @@ static int kiocb_cancel(struct aio_kiocb *kiocb)
> {
> kiocb_cancel_fn *cancel = kiocb->ki_cancel;
>
> + list_del_init(&kiocb->ki_list);
> +
> if (!cancel)
> return -EINVAL;
> kiocb->ki_cancel = NULL;
> @@ -607,8 +609,6 @@ static void free_ioctx_users(struct percpu_ref *ref)
> while (!list_empty(&ctx->active_reqs)) {
> req = list_first_entry(&ctx->active_reqs,
> struct aio_kiocb, ki_list);
> -
> - list_del_init(&req->ki_list);
> kiocb_cancel(req);
> }
>
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 5/9] aio: simplify cancellation
From: Darrick J. Wong @ 2018-03-21 16:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-6-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:28AM +0100, Christoph Hellwig wrote:
> With the current aio code there is no need for the magic KIOCB_CANCELLED
> value, as a cancelation just kicks the driver to queue the completion
> ASAP, with all actual completion handling done in another thread. Given
> that both the completion path and cancelation take the context lock there
> is no need for magic cmpxchg loops either.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> fs/aio.c | 37 +++++++++----------------------------
> 1 file changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index c32c315f05b5..2d40cf5dd4ec 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -156,19 +156,6 @@ struct kioctx {
> unsigned id;
> };
>
> -/*
> - * We use ki_cancel == KIOCB_CANCELLED to indicate that a kiocb has been either
> - * cancelled or completed (this makes a certain amount of sense because
> - * successful cancellation - io_cancel() - does deliver the completion to
> - * userspace).
> - *
> - * And since most things don't implement kiocb cancellation and we'd really like
> - * kiocb completion to be lockless when possible, we use ki_cancel to
> - * synchronize cancellation and completion - we only set it to KIOCB_CANCELLED
> - * with xchg() or cmpxchg(), see batch_complete_aio() and kiocb_cancel().
> - */
> -#define KIOCB_CANCELLED ((void *) (~0ULL))
> -
> struct aio_kiocb {
> union {
> struct kiocb rw;
> @@ -565,24 +552,18 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> }
> EXPORT_SYMBOL(kiocb_set_cancel_fn);
>
> +/*
> + * Only cancel if there ws a ki_cancel function to start with, and we
> + * are the one how managed to clear it (to protect against simulatinious
^^^ ^^^^^^^^^^^^^
I have the same complaint about how/who confusion and the spelling error
in this comment, but otherwise looks fine...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> + * cancel calls).
> + */
> static int kiocb_cancel(struct aio_kiocb *kiocb)
> {
> - kiocb_cancel_fn *old, *cancel;
> -
> - /*
> - * Don't want to set kiocb->ki_cancel = KIOCB_CANCELLED unless it
> - * actually has a cancel function, hence the cmpxchg()
> - */
> -
> - cancel = READ_ONCE(kiocb->ki_cancel);
> - do {
> - if (!cancel || cancel == KIOCB_CANCELLED)
> - return -EINVAL;
> -
> - old = cancel;
> - cancel = cmpxchg(&kiocb->ki_cancel, old, KIOCB_CANCELLED);
> - } while (cancel != old);
> + kiocb_cancel_fn *cancel = kiocb->ki_cancel;
>
> + if (!cancel)
> + return -EINVAL;
> + kiocb->ki_cancel = NULL;
> return cancel(&kiocb->rw);
> }
>
> --
> 2.14.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 8/9] aio: implement io_pgetevents
From: Greg KH @ 2018-03-21 14:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321092925.GA7265@lst.de>
On Wed, Mar 21, 2018 at 10:29:26AM +0100, Christoph Hellwig wrote:
> On Wed, Mar 21, 2018 at 10:24:43AM +0100, Greg KH wrote:
> > On Wed, Mar 21, 2018 at 08:32:31AM +0100, Christoph Hellwig wrote:
> > > This is the io_getevents equivalent of ppoll/pselect and allows to
> > > properly mix signals and aio completions (especially with IOCB_CMD_POLL)
> > > and atomically executes the following sequence:
> > >
> > > sigset_t origmask;
> > >
> > > pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
> > > ret = io_getevents(ctx, min_nr, nr, events, timeout);
> > > pthread_sigmask(SIG_SETMASK, &origmask, NULL);
> > >
> > > Note that unlike many other signal related calls we do not pass a sigmask
> > > size, as that would get us to 7 arguments, which aren't easily supported
> > > by the syscall infrastructure. It seems a lot less painful to just add a
> > > new syscall variant in the unlikely case we're going to increase the
> > > sigset size.
> >
> > Do we have a manpage for this new syscall and maybe a test program for
> > it so we can exercise it as part of the kselftests?
>
> The man page and test cases where submitted to libaio:
>
> http://git.infradead.org/users/hch/libaio.git/shortlog/refs/heads/aio-poll
>
> In the meantime the man page apparently moved to man-pages, I'll do that
> work once we make some forward progress.
Great!
> > And do we really need a compat thunk for a new syscall? Ugh, I guess
> > it's needed due to the long mess, right? No way to just define it the
> > same way for both arch sizes?
>
> Not without making it a pain to use. It should be a drop-in enhancement
> to the existing aio abis, which all work on these types.
Ok, make sense, thanks.
greg k-h
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 9/9] aio: implement IOCB_CMD_FSYNC and IOCB_CMD_FDSYNC
From: Christoph Hellwig @ 2018-03-21 9:30 UTC (permalink / raw)
To: Greg KH
Cc: Christoph Hellwig, viro, Avi Kivity, linux-aio, linux-fsdevel,
linux-api, linux-kernel
In-Reply-To: <20180321092713.GR14085@kroah.com>
On Wed, Mar 21, 2018 at 10:27:13AM +0100, Greg KH wrote:
> I hate the "bool" arguments to functions as you always need to go back
> and look them up. A "wrapper" of "aio_fsync_datasync()" and
> "aio_fsync_nodatasync()" around this maybe?
This is how the fsync API works. Wrappers really don't help anything,
although flags would certainly be nicer. Next time I need to touch
->fsync for one reason or another that is high on the todo list.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 8/9] aio: implement io_pgetevents
From: Christoph Hellwig @ 2018-03-21 9:29 UTC (permalink / raw)
To: Greg KH
Cc: Christoph Hellwig, viro, Avi Kivity, linux-aio, linux-fsdevel,
linux-api, linux-kernel
In-Reply-To: <20180321092443.GQ14085@kroah.com>
On Wed, Mar 21, 2018 at 10:24:43AM +0100, Greg KH wrote:
> On Wed, Mar 21, 2018 at 08:32:31AM +0100, Christoph Hellwig wrote:
> > This is the io_getevents equivalent of ppoll/pselect and allows to
> > properly mix signals and aio completions (especially with IOCB_CMD_POLL)
> > and atomically executes the following sequence:
> >
> > sigset_t origmask;
> >
> > pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
> > ret = io_getevents(ctx, min_nr, nr, events, timeout);
> > pthread_sigmask(SIG_SETMASK, &origmask, NULL);
> >
> > Note that unlike many other signal related calls we do not pass a sigmask
> > size, as that would get us to 7 arguments, which aren't easily supported
> > by the syscall infrastructure. It seems a lot less painful to just add a
> > new syscall variant in the unlikely case we're going to increase the
> > sigset size.
>
> Do we have a manpage for this new syscall and maybe a test program for
> it so we can exercise it as part of the kselftests?
The man page and test cases where submitted to libaio:
http://git.infradead.org/users/hch/libaio.git/shortlog/refs/heads/aio-poll
In the meantime the man page apparently moved to man-pages, I'll do that
work once we make some forward progress.
> And do we really need a compat thunk for a new syscall? Ugh, I guess
> it's needed due to the long mess, right? No way to just define it the
> same way for both arch sizes?
Not without making it a pain to use. It should be a drop-in enhancement
to the existing aio abis, which all work on these types.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 9/9] aio: implement IOCB_CMD_FSYNC and IOCB_CMD_FDSYNC
From: Greg KH @ 2018-03-21 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-10-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:32AM +0100, Christoph Hellwig wrote:
> Simple workqueue offload for now, but prepared for adding a real aio_fsync
> method if the need arises. Based on an earlier patch from Dave Chinner.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/aio.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index da87cbf7c67a..79d3eb3d2dd9 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -156,9 +156,16 @@ struct kioctx {
> unsigned id;
> };
>
> +struct fsync_iocb {
> + struct work_struct work;
> + struct file *file;
> + bool datasync;
> +};
> +
> struct aio_kiocb {
> union {
> struct kiocb rw;
> + struct fsync_iocb fsync;
> };
>
> struct kioctx *ki_ctx;
> @@ -1565,6 +1572,43 @@ static ssize_t aio_write(struct kiocb *req, struct iocb *iocb, bool vectored,
> return ret;
> }
>
> +static void aio_fsync_work(struct work_struct *work)
> +{
> + struct fsync_iocb *req = container_of(work, struct fsync_iocb, work);
> + int ret;
> +
> + ret = vfs_fsync(req->file, req->datasync);
> + fput(req->file);
> + aio_complete(container_of(req, struct aio_kiocb, fsync), ret, 0);
> +}
> +
> +static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
I hate the "bool" arguments to functions as you always need to go back
and look them up. A "wrapper" of "aio_fsync_datasync()" and
"aio_fsync_nodatasync()" around this maybe?
Anyway, very tiny nit, not a big deal, it's your code, you can maintain
it as-is :)
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 8/9] aio: implement io_pgetevents
From: Greg KH @ 2018-03-21 9:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-9-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:31AM +0100, Christoph Hellwig wrote:
> This is the io_getevents equivalent of ppoll/pselect and allows to
> properly mix signals and aio completions (especially with IOCB_CMD_POLL)
> and atomically executes the following sequence:
>
> sigset_t origmask;
>
> pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
> ret = io_getevents(ctx, min_nr, nr, events, timeout);
> pthread_sigmask(SIG_SETMASK, &origmask, NULL);
>
> Note that unlike many other signal related calls we do not pass a sigmask
> size, as that would get us to 7 arguments, which aren't easily supported
> by the syscall infrastructure. It seems a lot less painful to just add a
> new syscall variant in the unlikely case we're going to increase the
> sigset size.
Do we have a manpage for this new syscall and maybe a test program for
it so we can exercise it as part of the kselftests?
And do we really need a compat thunk for a new syscall? Ugh, I guess
it's needed due to the long mess, right? No way to just define it the
same way for both arch sizes?
Anyway, the code seems sane to me:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 7/9] aio: add delayed cancel support
From: Greg KH @ 2018-03-21 9:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-8-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:30AM +0100, Christoph Hellwig wrote:
> The upcoming aio poll support would like to be able to complete the
> iocb inline from the cancellation context, but that would cause
> a lock order reversal. Add support for optionally moving the cancelation
> outside the context lock to avoid this reversal.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> fs/aio.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 0b6394b4e528..9d7d6e4cde87 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -170,6 +170,10 @@ struct aio_kiocb {
> struct list_head ki_list; /* the aio core uses this
> * for cancellation */
>
> + unsigned int flags; /* protected by ctx->ctx_lock */
> +#define AIO_IOCB_DELAYED_CANCEL (1 << 0)
> +#define AIO_IOCB_CANCELLED (1 << 1)
BIT(0) and BIT(1)?
Anyway, not a big deal...
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 2/9] aio: remove an outdated comment in aio_complete
From: Christoph Hellwig @ 2018-03-21 9:17 UTC (permalink / raw)
To: Greg KH
Cc: Christoph Hellwig, viro, Avi Kivity, linux-aio, linux-fsdevel,
linux-api, linux-kernel
In-Reply-To: <20180321091420.GJ14085@kroah.com>
On Wed, Mar 21, 2018 at 10:14:20AM +0100, Greg KH wrote:
> > diff --git a/fs/aio.c b/fs/aio.c
> > index 03d59593912d..41fc8ce6bc7f 100644
> > --- a/fs/aio.c
> > +++ b/fs/aio.c
> > @@ -1088,6 +1088,8 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
> > unsigned tail, pos, head;
> > unsigned long flags;
> >
> > + BUG_ON(is_sync_kiocb(kiocb));
>
> Is this BUG_ON even needed anymore? Does it ever trip in any "regular"
> use, or is it only there for when a developer does something dumb? If
> "dumb", then we should keep it, otherwise we might be able to just drop
> it.
Probably not event needed anymore. is_sync_kiocb checks that
ki_complete is non-NULL, and aio_complete is static in aio.c now, so
it is almost impossibly to hit.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 6/9] aio: delete iocbs from the active_reqs list in kiocb_cancel
From: Greg KH @ 2018-03-21 9:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-7-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:29AM +0100, Christoph Hellwig wrote:
> One we cancel an iocb there is no reason to keep it on the active_reqs
> list, given that the list is only used to look for cancelation candidates.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 5/9] aio: simplify cancellation
From: Greg KH @ 2018-03-21 9:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-6-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:28AM +0100, Christoph Hellwig wrote:
> With the current aio code there is no need for the magic KIOCB_CANCELLED
> value, as a cancelation just kicks the driver to queue the completion
> ASAP, with all actual completion handling done in another thread. Given
> that both the completion path and cancelation take the context lock there
> is no need for magic cmpxchg loops either.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 4/9] aio: sanitize ki_list handling
From: Greg KH @ 2018-03-21 9:16 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-5-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:27AM +0100, Christoph Hellwig wrote:
> Instead of handcoded non-null checks always initialize ki_list to an
> empty list and use list_empty / list_empty_careful on it. While we're
> at it also error out on a double call to kiocb_set_cancel_fn instead
> of ignoring it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/aio.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 6295fc00f104..c32c315f05b5 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -555,13 +555,12 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> struct kioctx *ctx = req->ki_ctx;
> unsigned long flags;
>
> - spin_lock_irqsave(&ctx->ctx_lock, flags);
> -
> - if (!req->ki_list.next)
> - list_add(&req->ki_list, &ctx->active_reqs);
> + if (WARN_ON_ONCE(!list_empty(&req->ki_list)))
> + return;
Will be fun to see if the syzkaller code ends up tripping on this one,
their use of panic-on-warn always is "interesting"...
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> + spin_lock_irqsave(&ctx->ctx_lock, flags);
> + list_add_tail(&req->ki_list, &ctx->active_reqs);
> req->ki_cancel = cancel;
> -
> spin_unlock_irqrestore(&ctx->ctx_lock, flags);
> }
> EXPORT_SYMBOL(kiocb_set_cancel_fn);
> @@ -1034,7 +1033,7 @@ static inline struct aio_kiocb *aio_get_req(struct kioctx *ctx)
> goto out_put;
>
> percpu_ref_get(&ctx->reqs);
> -
> + INIT_LIST_HEAD(&req->ki_list);
> req->ki_ctx = ctx;
> return req;
> out_put:
> @@ -1080,7 +1079,7 @@ static void aio_complete(struct aio_kiocb *iocb, long res, long res2)
> unsigned tail, pos, head;
> unsigned long flags;
>
> - if (iocb->ki_list.next) {
> + if (!list_empty_careful(iocb->ki_list.next)) {
> unsigned long flags;
>
> spin_lock_irqsave(&ctx->ctx_lock, flags);
> --
> 2.14.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 3/9] aio: refactor read/write iocb setup
From: Greg KH @ 2018-03-21 9:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-4-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:26AM +0100, Christoph Hellwig wrote:
> Don't reference the kiocb structure from the common aio code, and move
> any use of it into helper specific to the read/write path. This is in
> preparation for aio_poll support that wants to use the space for different
> fields.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/aio.c | 171 ++++++++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 97 insertions(+), 74 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 41fc8ce6bc7f..6295fc00f104 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -170,7 +170,9 @@ struct kioctx {
> #define KIOCB_CANCELLED ((void *) (~0ULL))
>
> struct aio_kiocb {
> - struct kiocb common;
> + union {
> + struct kiocb rw;
> + };
>
> struct kioctx *ki_ctx;
> kiocb_cancel_fn *ki_cancel;
> @@ -549,7 +551,7 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events)
>
> void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> {
> - struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, common);
> + struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
> struct kioctx *ctx = req->ki_ctx;
> unsigned long flags;
>
> @@ -582,7 +584,7 @@ static int kiocb_cancel(struct aio_kiocb *kiocb)
> cancel = cmpxchg(&kiocb->ki_cancel, old, KIOCB_CANCELLED);
> } while (cancel != old);
>
> - return cancel(&kiocb->common);
> + return cancel(&kiocb->rw);
> }
>
> static void free_ioctx(struct work_struct *work)
> @@ -1040,15 +1042,6 @@ static inline struct aio_kiocb *aio_get_req(struct kioctx *ctx)
> return NULL;
> }
>
> -static void kiocb_free(struct aio_kiocb *req)
> -{
> - if (req->common.ki_filp)
> - fput(req->common.ki_filp);
> - if (req->ki_eventfd != NULL)
> - eventfd_ctx_put(req->ki_eventfd);
> - kmem_cache_free(kiocb_cachep, req);
> -}
> -
> static struct kioctx *lookup_ioctx(unsigned long ctx_id)
> {
> struct aio_ring __user *ring = (void __user *)ctx_id;
> @@ -1079,29 +1072,14 @@ static struct kioctx *lookup_ioctx(unsigned long ctx_id)
> /* aio_complete
> * Called when the io request on the given iocb is complete.
> */
> -static void aio_complete(struct kiocb *kiocb, long res, long res2)
> +static void aio_complete(struct aio_kiocb *iocb, long res, long res2)
> {
> - struct aio_kiocb *iocb = container_of(kiocb, struct aio_kiocb, common);
> struct kioctx *ctx = iocb->ki_ctx;
> struct aio_ring *ring;
> struct io_event *ev_page, *event;
> unsigned tail, pos, head;
> unsigned long flags;
>
> - BUG_ON(is_sync_kiocb(kiocb));
Ah, nevermind about my previous email, sorry, should have kept
reading...
> +static void aio_complete_rw(struct kiocb *kiocb, long res, long res2)
> +{
> + struct aio_kiocb *iocb = container_of(kiocb, struct aio_kiocb, rw);
> +
> + WARN_ON_ONCE(is_sync_kiocb(kiocb));
That's nicer, thanks.
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 2/9] aio: remove an outdated comment in aio_complete
From: Greg KH @ 2018-03-21 9:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-3-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:25AM +0100, Christoph Hellwig wrote:
> These days we don't treat sync iocbs special in the aio completion code as
> they never use it. Remove the old comment, and move the BUG_ON for a sync
> iocb to the top of the function.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/aio.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 03d59593912d..41fc8ce6bc7f 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1088,6 +1088,8 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
> unsigned tail, pos, head;
> unsigned long flags;
>
> + BUG_ON(is_sync_kiocb(kiocb));
Is this BUG_ON even needed anymore? Does it ever trip in any "regular"
use, or is it only there for when a developer does something dumb? If
"dumb", then we should keep it, otherwise we might be able to just drop
it.
Either way, this patch is fine:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 1/9] aio: don't print the page size at boot time
From: Greg KH @ 2018-03-21 9:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180321073232.13366-2-hch@lst.de>
On Wed, Mar 21, 2018 at 08:32:24AM +0100, Christoph Hellwig wrote:
> The page size is in no way related to the aio code, and printing it in
> the (debug) dmesg at every boot serves no purpose.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 12/28] net: convert datagram_poll users tp ->poll_mask
From: Greg KH @ 2018-03-21 9:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-13-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:16AM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/isdn/mISDN/socket.c | 2 +-
> drivers/net/ppp/pppoe.c | 2 +-
> drivers/staging/ipx/af_ipx.c | 2 +-
> drivers/staging/irda/net/af_irda.c | 6 +++---
irda is now gone in linux-next, but that's an easy thing to handle when
merging.
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 28/28] random: convert to ->poll_mask
From: Greg KH @ 2018-03-21 9:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-29-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:32AM +0100, Christoph Hellwig wrote:
> The big change is that random_read_wait and random_write_wait are merged
> into a single waitqueue that uses keyed wakeups. Because wait_event_*
> doesn't know about that this will lead to occassional spurious wakeups
> in _random_read and add_hwgenerator_randomness, but wait_event_* is
> designed to handle these and were are not in a a hot path there.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/char/random.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL
From: Greg KH @ 2018-03-21 9:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180321074032.14211-7-hch@lst.de>
On Wed, Mar 21, 2018 at 08:40:10AM +0100, Christoph Hellwig wrote:
> Simple one-shot poll through the io_submit() interface. To poll for
> a file descriptor the application should submit an iocb of type
> IOCB_CMD_POLL. It will poll the fd for the events specified in the
> the first 32 bits of the aio_buf field of the iocb.
>
> Unlike poll or epoll without EPOLLONESHOT this interface always works
> in one shot mode, that is once the iocb is completed, it will have to be
> resubmitted.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox