Re: [PATCH 7/7] um: simplify IRQ handling code

[Johannes Berg <johannes@sipsolutions.net>]

Sorry, looks like I forgot to reply to this earlier.

On Tue, 2020-11-24 at 22:36 +0000, Anton Ivanov wrote:

> > > > @@ -151,73 +171,33 @@ static int activate_fd(int irq, int fd, enum um_irq_type type, void *dev_id)
> > > >    		goto out;
> > > >    
> > > >    	spin_lock_irqsave(&irq_lock, flags);
> > > > -
> > > > -	/* Check if we have an entry for this fd */
> > > > -
> > > > -	err = -EBUSY;
> > > > -	for (irq_entry = active_fds;
> > > > -		irq_entry != NULL; irq_entry = irq_entry->next) {
> > > > -		if (irq_entry->fd == fd)
> > > > -			break;
> > > > -	}
> > > > -
> > > > -	if (irq_entry == NULL) {
> > > > -		/* This needs to be atomic as it may be called from an
> > > > -		 * IRQ context.
> > > > -		 */
> > > > -		irq_entry = kmalloc(sizeof(struct irq_entry), GFP_ATOMIC);
> > > > -		if (irq_entry == NULL) {
> > > > -			printk(KERN_ERR
> > > > -				"Failed to allocate new IRQ entry\n");
> > > > +	irq_entry = get_irq_entry_by_fd(fd);
> > > > +	if (irq_entry) {
> > > 
> > > This is not right.
> > > 
> > > You can re-register the same IRQ/fd, but with a different mask - f.e.
> > > turn on/off write or read on the same fd. F.E. - you have registered a
> > > read IRQ, you after that register same IRQ for write and you can alter
> > > the mask.
> > 
> > Hmm. You snipped some code, and it continued like this:
> > 
> >          irq_entry = get_irq_entry_by_fd(fd);
> >          if (irq_entry) {
> >                  /* cannot register the same FD twice with the same type */
> >                  if (WARN_ON(irq_entry->reg[type].events)) {
> >                          // basically return -EALREADY
> > 
> > 
> > I'm not sure I see this is different from what it was before? If the
> 
> The original intention was to be able to do it :)

To do _what_ exactly? You said to re-register the same FD, but you could
do that before by unregistering?

And read/write are completely separate entries anyway. You cannot do
both at the same time (they're not bitmasks, just enum values.)

In fact, you *can* still use the same FD for both, with and without this
patch. We find the irq_entry here (by FD), and then check if this type
of event is already used. That means you cannot register for READ for
the same FD twice, but using the other event (WRITE) is still fine.

johannes


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um