Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754585AbcCJRVF (ORCPT ); Thu, 10 Mar 2016 12:21:05 -0500 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:59441 "EHLO e23smtp06.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753653AbcCJRUu convert rfc822-to-8bit (ORCPT ); Thu, 10 Mar 2016 12:20:50 -0500 X-IBM-Helo: d23dlp01.au.ibm.com X-IBM-MailFrom: vaibhav@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org From: Vaibhav Jain To: Frederic Barrat , Ian Munsie , Michael Ellerman , linux-kernel , Matt Ochs , Manoj Kumar Cc: linuxppc-dev , Michael Neuling Subject: Re: [PATCH v3 1/2] cxl: Add mechanism for delivering AFU driver specific events In-Reply-To: <56E05876.6090006@linux.vnet.ibm.com> References: <1457401715-26435-1-git-send-email-imunsie@au.ibm.com> <87a8m7iunv.fsf@vajain21.in.ibm.com> <56E05876.6090006@linux.vnet.ibm.com> User-Agent: Notmuch/0.21+45~ga5c1536 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-unknown-linux-gnu) Date: Thu, 10 Mar 2016 22:49:49 +0530 Message-ID: <87io0up7wq.fsf@vajain21.in.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16031017-0021-0000-0000-000002F313C7 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5794 Lines: 189 Frederic Barrat writes: > Hi Vaibhav, > > Le 09/03/2016 15:37, Vaibhav Jain a écrit : > >> I would propose these two apis. >> >> /* >> * fetches an event from the driver event queue. NULL means that queue >> * is empty. Can sleep if needed. The memory for cxl_event is allocated >> * by module being called. Hence it can be potentially be larger then >> * sizeof(struct cxl_event). Multiple calls to this should return same >> * pointer untill ack_event is called. >> */ >> struct cxl_event * fetch_event(struct cxl_context * ctx); >> >> /* >> * Returns and acknowledge the struct cxl_event * back to the driver >> * which can then free it or maybe put it back in a kmem_cache. This >> * should be called once we have completely returned the current >> * struct cxl_event from the readcall >> */ >> void ack_event(struct cxl_context * ctx, struct cxl_event *); > > > How would you implement polling on those APIs? Hi Fred. I am looking at an implementation similar to this: static inline bool ctx_event_pending(struct cxl_context *ctx) { typeof (ctx->afu_driver_ops->fetch_event) fn_events = (ctx->afu_driver_ops != NULL) ? ctx->afu_driver_ops->fetch_event : NULL; if (ctx->pending_irq || ctx->pending_fault || ctx->pending_afu_err) return true; /* * if fn_event returns a not null then its gauranteed to return * the same pointer on next call */ if (fn_events) return fn_events(ctx) != NULL; return false; } unsigned int afu_poll(struct file *file, struct poll_table_struct *poll) { struct cxl_context *ctx = file->private_data; int mask = 0; unsigned long flags; poll_wait(file, &ctx->wq, poll); pr_devel("afu_poll wait done pe: %i\n", ctx->pe); spin_lock_irqsave(&ctx->lock, flags); if (ctx_event_pending(ctx)) mask |= POLLIN | POLLRDNORM; else if (ctx->status == CLOSED) /* Only error on closed when there are no futher events pending */ mask |= POLLERR; spin_unlock_irqrestore(&ctx->lock, flags); pr_devel("afu_poll pe: %i returning %#x\n", ctx->pe, mask); return mask; } > How would you implement afu_read? There are several sources of events. Looking at an implementation similar to this: ssize_t afu_read(struct file *file, char __user *buf, size_t count, loff_t *off) { unsigned long flags; ssize_t rc = 0; struct cxl_context *ctx = file->private_data; struct cxl_event *ptr_event, event = { .header.process_element = ctx->pe, .header.size = sizeof(struct cxl_event_header) }; typeof (ctx->afu_driver_ops->fetch_event) fn_fetch_event = (ctx->afu_driver_ops != NULL) ? ctx->afu_driver_ops->fetch_event : NULL; typeof (ctx->afu_driver_ops->ack_event) fn_ack_event = (ctx->afu_driver_ops != NULL) ? ctx->afu_driver_ops->ack_event : NULL; if (count < CXL_READ_MIN_SIZE) return -EINVAL; if (!cxl_adapter_link_ok(ctx->afu->adapter) || ctx->status == CLOSED) return -EIO; if (signal_pending(current)) return -ERESTARTSYS; /* if no events then wait */ if (!ctx_event_pending(ctx)) { if ((file->f_flags & O_NONBLOCK)) return -EAGAIN; pr_devel("afu_read going to sleep...\n"); rc = wait_event_interruptible(ctx->wq, (ctx->status == CLOSED) || cxl_adapter_link_ok(ctx->afu->adapter) || ctx_event_pending(ctx)); pr_devel("afu_read woken up\n"); } /* did we get interrupted during wait sleep */ if (rc) return rc; /* get driver events if any */ ptr_event = fn_fetch_event ? fn_fetch_event(ctx) : NULL; /* In case of error feching driver specific event */ if (IS_ERR(ptr_event)) { pr_warn("Error fetching driver specific event %ld", PTR_ERR(ptr_event)); ptr_event = NULL; } /* code below manipulates ctx so take a spin lock */ spin_lock_irqsave(&ctx->lock, flags); /* give driver events first priority */ if (ptr_event) { pr_devel("afu_read delivering AFU driver specific event\n"); /* populate the header type and pe in the event struct */ ptr_event->header.type = CXL_EVENT_AFU_DRIVER; ptr_event->header.process_element = ctx->pe; WARN_ON(event.header.size > count); } else if (ctx->pending_irq) { pr_devel("afu_read delivering AFU interrupt\n"); event.header.size += sizeof(struct cxl_event_afu_interrupt); event.header.type = CXL_EVENT_AFU_INTERRUPT; event.irq.irq = find_first_bit(ctx->irq_bitmap, ctx->irq_count) + 1; clear_bit(event.irq.irq - 1, ctx->irq_bitmap); if (bitmap_empty(ctx->irq_bitmap, ctx->irq_count)) ctx->pending_irq = false; } else if (ctx->pending_fault) { pr_devel("afu_read delivering data storage fault\n"); event.header.size += sizeof(struct cxl_event_data_storage); event.header.type = CXL_EVENT_DATA_STORAGE; event.fault.addr = ctx->fault_addr; event.fault.dsisr = ctx->fault_dsisr; ctx->pending_fault = false; } else if (ctx->pending_afu_err) { pr_devel("afu_read delivering afu error\n"); event.header.size += sizeof(struct cxl_event_afu_error); event.header.type = CXL_EVENT_AFU_ERROR; event.afu_error.error = ctx->afu_err; ctx->pending_afu_err = false; } else if (ctx->status == CLOSED) { pr_devel("afu_read fatal error\n"); rc = -EIO; } else WARN(1, "afu_read must be buggy\n"); spin_unlock_irqrestore(&ctx->lock, flags); if (!rc) { /* if we dont have a driver event then use 'event' var */ ptr_event = ptr_event ? ptr_event : &event; rc = min(((size_t)ptr_event->header.size), count); if (copy_to_user(buf, ptr_event, rc)) rc = -EFAULT; } /* if its a driver event ack it back to the driver */ if (fn_ack_event && (ptr_event != &event)) fn_ack_event(ctx, ptr_event); return rc; } Cheers, ~ Vaibhav