Dave,
Yes, this was brought up in July of 1999:
Ole says (see also below, CAPS mine):
>From my old STREAMS Programmer's Guide: "Pointers to the open and close
>procedures must be contained in the read qinit. THESE FIELDS ARE
>IGNORED IN THE WRITE SIDE."
Lis does not ignore these fields, and is in error.
It's an official bug.
If you would have agreed to change it back in July 1999, we wouldn't be
having this discussion. That's when I started splitting qinit structures.
So why not fix it now and be done with it?
--brian
>From: Ole Husgaard <[EMAIL PROTECTED]>
>To: [EMAIL PROTECTED]
>Cc: [EMAIL PROTECTED], [EMAIL PROTECTED]
>Subject: Re: Two times close?
>Sender: [EMAIL PROTECTED]
>Precedence: bulk
>Status: RO
>Content-Length: 1831
>Lines: 49
>
>
>On Thu, 8 Jul 1999 13:36:11 -0500, Fabio Ferrari wrote:
>> I saw this code in LIS-2.4, and I can't understand why it is trying
>>to do "close" two times.
>> Isn't that a bug? The MOD_DEC_USE_COUNT inside the "close"
>>call should be running two times today, and if you have to free memory
>>in the "close", you should be doing it twice!
>
>From my old STREAMS Programmer's Guide: "Pointers to the open and close
>procedures must be contained in the read qinit. These fields are
>ignored in the write side."
>
>If your close routine in the write qinit structure is NULL, you get
>around this bug.
>
>>Take a look at head/head.c#1131:
>>
>> for (i = 1; i <= 2; i++, q = LIS_WR(q)) /* do twice */
>> {
>> if (!LIS_CHECK_Q_MAGIC(q)) return ; /* lose the memory */
>> if (LIS_DEBUG_CLOSE) name = lis_queue_name(q) ;
>> if (do_close && q->q_qinfo != NULL && q->q_qinfo->qi_qclose != NULL)
>> {
>> if (name)
>> printk("lis_qdetach: queue \"%s\": calling close routine\n",
>> name);
>>
>> CLOCKADD() ; /* user routine might sleep */
>> (*q->q_qinfo->qi_qclose)(q, (q->q_next ? 0 : flag), creds);
>> CLOCKON() ;
>> }
>> }
>
>The second bug I see here is that the close timer is not stopped if
>this code encounters a queue with bad magic.
>I do not like this code: Checking for a close function and calling it
>should be done for the read queue only. But then we could just as well
>do without the loop and the variable i. And doing "q = LIS_WR(q)" two
>times is IMHO ugly even if it works under LiS.
>I wonder if we have to call lis_queue_name() for the write queue too.
>
>I would prefer to wait for a comment from Dave (he is on vacation)
>before changing the code.
>
>
>Best regards,
>Ole Husgaard
>[EMAIL PROTECTED]
On Fri, 10 Oct 2003, Dave Grothe wrote:
> In all my drivers I use separate qinit structures for read and write
> queues. It makes sense since one naturally would specify xxx_wput and
> xxx_rput differently. Just supply a NULL pointer for the close routine on
> the write-side qinit and you will only get one call to close.
> -- Dave
>
> At 02:35 PM 10/10/2003 Friday, David Lehmann wrote:
> >David Grothe wrote:
> >>David:
> >>I think I see what is going on. Consider the following lines from the trace:
> >>...
> >>Queues come in pairs. This loop is examining each queue of the pair and
> >>calling the close routine for whichever queue has a pointer to a close
> >>routine. Apparently your qinfo structure for your module has a pointer
> >>to the close routine for both the read and write queues. It is
> >>conventional to only provide open/close routine pointers for the read queue.
> >>I think if you change your qinit structure the problem will go away.
> >
> >Maybe so, but this is the same code that Solaris uses.
> >My understanding is that close should be called once and only once
> >for the queue pair. Maybe you should take John's suggestion and
> >set do_close to zero after executing the close routine once.
> >
> >--
> >
> >David Lehmann Ulticom, Inc.
> >AOL/Yahoo IM: davidULCM 1020 Briggs Road
> >1-856-787-2729 Mt. Laurel, NJ 08054 USA
> >
> >
> >_______________________________________________
> >Linux-streams mailing list
> >[EMAIL PROTECTED]
> >http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
>
>
> _______________________________________________
> Linux-streams mailing list
> [EMAIL PROTECTED]
> http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
--
Brian F. G. Bidulock � The reasonable man adapts himself to the �
[EMAIL PROTECTED] � world; the unreasonable one persists in �
http://www.openss7.org/ � trying to adapt the world to himself. �
� Therefore all progress depends on the �
� unreasonable man. -- George Bernard Shaw �
_______________________________________________
Linux-streams mailing list
[EMAIL PROTECTED]
http://gsyc.escet.urjc.es/mailman/listinfo/linux-streams
