Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753816AbdCBAwt (ORCPT ); Wed, 1 Mar 2017 19:52:49 -0500 Received: from quartz.orcorp.ca ([184.70.90.242]:57361 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753375AbdCBAwp (ORCPT ); Wed, 1 Mar 2017 19:52:45 -0500 Date: Wed, 1 Mar 2017 17:50:47 -0700 From: Jason Gunthorpe To: Logan Gunthorpe Cc: Bjorn Helgaas , Keith Busch , Myron Stowe , Greg Kroah-Hartman , Bjorn Helgaas , Geert Uytterhoeven , Jonathan Corbet , "David S. Miller" , Andrew Morton , Emil Velikov , Mauro Carvalho Chehab , Guenter Roeck , Jarkko Sakkinen , Linus Walleij , Ryusuke Konishi , Stefan Berger , Wei Zhang , Kurt Schwemmer , Stephen Bates , linux-pci@vger.kernel.org, linux-doc@vger.kernel.org, linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 0/4] New Microsemi PCI Switch Management Driver Message-ID: <20170302005047.GA14301@obsidianresearch.com> References: <1488091997-12843-1-git-send-email-logang@deltatee.com> <20170301214120.GA30451@bhelgaas-glaptop.roam.corp.google.com> <4a867c32-ed29-bc98-c7cb-6315243e664a@deltatee.com> <20170301235854.GF2820@obsidianresearch.com> <7d6b3b59-5cec-d73f-b65c-4d685ccc70f8@deltatee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7d6b3b59-5cec-d73f-b65c-4d685ccc70f8@deltatee.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.156 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1764 Lines: 47 On Wed, Mar 01, 2017 at 05:23:38PM -0700, Logan Gunthorpe wrote: > > That could help, but this would mean cdev would have to insert a shim > > to grab locks around the various file ops. > > Hmm, I was hoping something more along the lines of actually killing the > processes instead of just shimming away fops. That would probably make most cdev users unhappy, it is not what we want in tpm or infiniband, for instance. > > AFAIK TPM is correct and has been robustly tested now. We have a 'vtpm' > > driver that agressively uses hot-unplug. > > Switchtec is a bit more tricky because a) there's no upper level driver > to handle things Introducing a light split between 'the upper part that owns the cdev' and 'the lower part that owns the hardware' makes things much easier to understand in a driver and it becomes clearer where, eg, devm actions should be linked (ie probably not to the cdev part) > and b) userspace may be inside a wait_for_completion (via read or > poll) that needs to be completed. If a so called 'cdev_kill' could > actually just kill these processes it would be a bit easier. For TPM, poll could be something like: static unsigned int tpm_poll(struct file *filp, struct poll_table_struct *wait) { poll_wait(filp, &chip->poll_wait, wait); if (tpm_try_get_ops(chip)) { mask = chip->ops->driver_do_poll(...); tpm_put_ops(chip); } else mask = POLLIN | POLLRDHUP | POLLOUT | POLLERR | POLLHUP; return mask; } And we would trigger chip->poll_wait in the unregister. wait_for_completion is similar, drop the rwsem while sleeping, add 'ops = NULL' to the sleeping condition test, trigger the wait on unregister then reacquire the rwsem and test ops on wake. Jason