Re: [PATCH 07/15] tgt: os.h: sync_file_range is OS dependent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux