On Tue, 23 Oct 2007, Boaz Harrosh wrote:Yes, that would be one clean situation. Well, I'd personally actually prefer to *not* have the count be passed down explicitly, because it's just too error prone. So I'd much rather see the count implicit in the list: whether it's in an explicit header structure (that is the *only* thing passed down) or whether it's embedded in the list itself is not important. Since the list itself has to have the "next pointer" for chaining, and thus already has "embedded information" in it, it actually does make sense in my opinion to just embed the end-of-list information too. And the end result right now is pretty simple, with "sg_next()" being really simple to use, and there being no way to screw things up by getting the count and the sg pointer out of sync. My biggest complaint right now is that a lot of users of the sg *filling* functions were mindlessly converted, so we have code like cryptoloop.c: sg_set_page(&sg_in, in_page); cryptoloop.c: sg_in.offset = in_offs; cryptoloop.c: sg_in.length = sz; which is just really stupid, and we should have a function for that. But worse is code like this: ub.c: sg_set_page(sg, virt_to_page(sc->top_sense)); ub.c: sg->offset = (unsigned long)sc->top_sense & (PAGE_SIZE-1); ub.c: sg->length = UB_SENSE_SIZE; which again was converted "line by line" and we actually *do* have a function to do the above three lines as sg_set_buf(sg, sc->top_sense, UB_SENSE_SIZE); where that *single* line is just tons shorter but more importantly, more readable, than the mess that is a brute-force conversion. So I think the SG stuff looks ok now, but I think we have a lot of "fix up the rough edges" to go! (The above is not the only case. Just grep for "sg_set_page", and you'll see several examples of this kind of hard-to-read code. Basically, I don't think it's ever a good idea to initialize the SG entries one by one, and even when we have a hard page/offset/size thing, we should not set them one by one, and we should probably extend sg_set_page() to always take offset and length too, since setting one without the other two is never really sensible!) Linus -
| Kristen Carlson Accardi | [PATCH] ata: ahci: power off unused ports |
| Trent Piepho | [PATCH] [POWERPC] Improve (in|out)_beXX() asm code |
| Jan Engelhardt | Re: Kernel Development & Objective-C |
| Chuck Ebbert | Why do so many machines need "noapic"? |
git: | |
| Jan Engelhardt | about c8af1de9 (git status uses pager) |
| Junio C Hamano | Re: [PATCH 3/3] Teach "git branch" about --new-workdir |
| Nicolas Pitre | Re: Git tree for old kernels from before the current tree |
| Linus Torvalds | Re: cleaner/better zlib sources? |
| Nick Guenther | Re: Real men don't attack straw men |
| Predrag Punosevac | Skype on the OpenBSD |
| Karsten McMinn | Re: Packets Per Second Limit? |
| L. V. Lammert | Re: About Xen: maybe a reiterative question but .. |
| Marnix Klooster | Re: Europe Distribution |
| Sean Moss-Pultz | [openmoko-announce] Let us impact the material world |
| Kosa | Re: Car Mode Application... |
| Federico Lorenzi | Re: multi-tutch? |
| Battery Maximizer Software | 7 hours ago | Linux kernel |
| windows folder creation surprise | 9 hours ago | Windows |
| Problem in scim in Fedora 9 | 10 hours ago | Linux general |
| Firewall | 1 day ago | OpenBSD |
| IP layer send packet | 1 day ago | Linux kernel |
| dtrace for linux available | 2 days ago | Linux kernel |
| Unable to mount ramdisk image using UBoot while upgrading to 2.6.15 kernel for a MPC8540 based target | 2 days ago | Linux kernel |
| RealTek RTL8169 - can't connect | 2 days ago | NetBSD |
| vsftpd Upload Problems | 2 days ago | Linux general |
| creating con folder in desktop | 3 days ago | Windows |
