On Mon, 02 Mar 2009 13:24:31 +0200 Or Gerlitz <ogerlitz@xxxxxxxxxxxx> wrote: > Boaz Harrosh wrote: > > Introduce a new header that includes definitions of all OS specific services. stgt code will use abstract API from os.h. (start with os_xxx). Then a linux/os.c and bsd/os.c implement these services for the needed platform. > > > > First such service is sync_file_range which needs to be emulated in bsd. > > usr/linux/os.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > > > Hi Boaz, > > I think best if we can avoid having a --linux-- os.c file. If not, at > least avoid adding one-hop for fast-path calls such as sync_file_range > (e.g in this case under linux let it be inline as it used to be). If we want to avoid linux/os.c, we need to use ifdef or __weak attribute. With the former, we can use inline but it doesn't look pretty (so we use linux/os.c). With the latter, it's pretty but we can't use inline and some buggy gcc versions can't handle __weak properly. So we have to choose inline or linux/os.c. I don't think any functions in linux/os.c is a fast-path call. I really don't think that making sync_file_range inline affects the performance. > Also, > I wasn't sure where the patch to bs_aio.c from the previous post (11/14) > landed in this sequence. I think: Subject: [PATCH 15/15] tgt: usr/Makefile BSD build Support -TGTD_OBJS += bs_rdwr.o bs_aio.o +TGTD_OBJS += bs_rdwr.o +ifneq (FreeBSD,$(UNAME)) +TGTD_OBJS += bs_aio.o +endif As I suggested, we compile bs_aio.o only with Linux. It makes sense because bs_aio is Linux specific AIO code. -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html