On Monday 13 June 2005 18:27, Joerg Sonnenberger wrote: > On Mon, Jun 13, 2005 at 05:58:24PM +0200, Hans Petter Selasky wrote: > > static void > > filter(struct fifo *f) > > { > > do { > > if(!f->len) > > { > > if(f->m) ...; > > > > f->m = get_mbuf(); > > if(!f->m) break; > > > > f->len = m->m_len; > > f->ptr = m->m_data; > > } > > > > /* f->Z_chip is the maximum transfer length */ > > > > io_len = min(f->len, f->Z_chip); > > if (io_len == 0) > continue; > > > bus_space_write_multi_1(t,h,xxx,f->ptr,io_len); > > > > f->len -= io_len; > > f->Z_chip -= io_len; > > f->ptr += io_len; > > > > } while(Z_chip); > > } > > [...] > > > Adding that extra check for zero transfer length is not going to affect > > performance at all. If one does that using "C", the compiler can optimize > > away that "if(count) ..." when "count" is a constant. Besides the i386 > > machine instructions "ins" and "outs" already exhibit that behaviour. > > The compiler can only optimize it away if it is known to be a constant. > But thinking again about it, it should be documented at least whether > a count of 0 is allowed or not. I think it makes more sense to not allow > it, but others (you included) might disagree.
Why? If a count of zero is not allowed, then "bus_space_xxx()" should panic on count equal to zero and not freeze the system, so that the user is able to find out what is wrong: #if defined(XXX) if(count == 0) panic("not allowed"); #endif But that is just stupid, why not just return: #if defined(XXX) if(count == 0) return; #endif And then I can put: #define XXX in my code before including "bus.h". Instead of creating wrappers, just to be sure: #define bus_space_read_multi_1(t,h,off,ptr,count) \ { if(count) bus_space_read_multi_1(t,h,off,ptr,count); } Because this is really specific to i386 and amd64. If you look at "alpha", "sparc64" and "ia64" they all use "while(count--) ...". So I think the one that wrote that code did a mistake. My theory is that he or her copied: static __inline void insb(u_int port, void *addr, size_t cnt) { __asm __volatile("cld; rep; insb" : "+D" (addr), "+c" (cnt) : "d" (port) : "memory"); } And forgot to add that extra check for count equal to zero, when converting "rep" into "loop"! --HPS _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"