diff --git a/src/common.h b/src/common.h index 2843005..2e545b6 100644 --- a/src/common.h +++ b/src/common.h @@ -69,6 +69,9 @@ extern std::string instance_name; #define REQUIRES(...) \ THREAD_ANNOTATION_ATTRIBUTE(requires_capability(__VA_ARGS__)) +#define RETURN_CAPABILITY(...) \ + THREAD_ANNOTATION_ATTRIBUTE(lock_returned(__VA_ARGS__)) + #define NO_THREAD_SAFETY_ANALYSIS \ THREAD_ANNOTATION_ATTRIBUTE(no_thread_safety_analysis) diff --git a/src/fdcache_entity.cpp b/src/fdcache_entity.cpp index a0b8938..7067b4e 100644 --- a/src/fdcache_entity.cpp +++ b/src/fdcache_entity.cpp @@ -29,6 +29,7 @@ #include "common.h" #include "fdcache_entity.h" +#include "fdcache_fdinfo.h" #include "fdcache_stat.h" #include "fdcache_untreated.h" #include "fdcache.h" @@ -1001,11 +1002,6 @@ bool FdEntity::SetAllStatus(bool is_loaded) if(-1 == physical_fd){ return false; } - // [NOTE] - // this method is only internal use, and calling after locking. - // so do not lock now. - // - //const std::lock_guard lock(fdent_lock); // get file size struct stat st{}; @@ -1410,9 +1406,6 @@ int FdEntity::RowFlushHasLock(int fd, const char* tpath, bool force_sync) return result; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// int FdEntity::RowFlushNoMultipart(const PseudoFdInfo* pseudo_obj, const char* tpath) { S3FS_PRN_INFO3("[tpath=%s][path=%s][pseudo_fd=%d][physical_fd=%d]", SAFESTRPTR(tpath), path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd); @@ -1475,9 +1468,6 @@ int FdEntity::RowFlushNoMultipart(const PseudoFdInfo* pseudo_obj, const char* tp return result; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// int FdEntity::RowFlushMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) { S3FS_PRN_INFO3("[tpath=%s][path=%s][pseudo_fd=%d][physical_fd=%d]", SAFESTRPTR(tpath), path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd); @@ -1581,9 +1571,6 @@ int FdEntity::RowFlushMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) return result; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// int FdEntity::RowFlushMixMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) { S3FS_PRN_INFO3("[tpath=%s][path=%s][pseudo_fd=%d][physical_fd=%d]", SAFESTRPTR(tpath), path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd); @@ -1708,9 +1695,6 @@ int FdEntity::RowFlushMixMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) return result; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// int FdEntity::RowFlushStreamMultipart(PseudoFdInfo* pseudo_obj, const char* tpath) { S3FS_PRN_INFO3("[tpath=%s][path=%s][pseudo_fd=%d][physical_fd=%d][mix_upload=%s]", SAFESTRPTR(tpath), path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd, (FdEntity::mixmultipart ? "true" : "false")); @@ -2059,9 +2043,6 @@ ssize_t FdEntity::Write(int fd, const char* bytes, off_t start, size_t size) return wsize; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// ssize_t FdEntity::WriteNoMultipart(const PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size) { S3FS_PRN_DBG("[path=%s][pseudo_fd=%d][physical_fd=%d][offset=%lld][size=%zu]", path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd, static_cast(start), size); @@ -2120,9 +2101,6 @@ ssize_t FdEntity::WriteNoMultipart(const PseudoFdInfo* pseudo_obj, const char* b return wsize; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// ssize_t FdEntity::WriteMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size) { S3FS_PRN_DBG("[path=%s][pseudo_fd=%d][physical_fd=%d][offset=%lld][size=%zu]", path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd, static_cast(start), size); @@ -2217,9 +2195,6 @@ ssize_t FdEntity::WriteMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, of return wsize; } -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// ssize_t FdEntity::WriteMixMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size) { S3FS_PRN_DBG("[path=%s][pseudo_fd=%d][physical_fd=%d][offset=%lld][size=%zu]", path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd, static_cast(start), size); @@ -2299,9 +2274,6 @@ ssize_t FdEntity::WriteMixMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, // On Stream upload, the uploading is executed in another thread when the // written area exceeds the maximum size of multipart upload. // -// [NOTE] -// Both fdent_lock and fdent_data_lock must be locked before calling. -// ssize_t FdEntity::WriteStreamUpload(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size) { S3FS_PRN_DBG("[path=%s][pseudo_fd=%d][physical_fd=%d][offset=%lld][size=%zu]", path.c_str(), (pseudo_obj ? pseudo_obj->GetPseudoFd() : -1), physical_fd, static_cast(start), size); @@ -2537,9 +2509,6 @@ void FdEntity::MarkDirtyMetadata() bool FdEntity::IsDirtyMetadata() const { - // [NOTE] - // fdent_lock must be previously locked. - // return (pending_status_t::UPDATE_META_PENDING == pending_status); } @@ -2555,11 +2524,6 @@ bool FdEntity::AddUntreated(off_t start, off_t size) return result; } -// [NOTE] -// An object that has already been locked with fdent_lock is passed to -// UploadBoundaryLastUntreatedArea(), which calls this method. -// Therefore, there is no need to lock fdent_lock in this method. -// bool FdEntity::GetLastUpdateUntreatedPart(off_t& start, off_t& size) const { // Get last untreated area @@ -2569,11 +2533,6 @@ bool FdEntity::GetLastUpdateUntreatedPart(off_t& start, off_t& size) const return true; } -// [NOTE] -// An object that has already been locked with fdent_lock is passed to -// UploadBoundaryLastUntreatedArea(), which calls this method. -// Therefore, there is no need to lock fdent_lock in this method. -// bool FdEntity::ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size) { if(0 < front_size){ diff --git a/src/fdcache_entity.h b/src/fdcache_entity.h index 972983f..a4da87a 100644 --- a/src/fdcache_entity.h +++ b/src/fdcache_entity.h @@ -28,10 +28,15 @@ #include "common.h" #include "fdcache_page.h" -#include "fdcache_fdinfo.h" #include "fdcache_untreated.h" #include "metaheader.h" +//---------------------------------------------- +// Typedef +//---------------------------------------------- +class PseudoFdInfo; +typedef std::map> fdinfo_map_t; + //------------------------------------------------ // class FdEntity //------------------------------------------------ @@ -55,14 +60,7 @@ class FdEntity mutable std::mutex fdent_lock; std::string path GUARDED_BY(fdent_lock); // object path int physical_fd GUARDED_BY(fdent_lock); // physical file(cache or temporary file) descriptor - - // [FIXME] - // GetLastUpdateUntreatedPart and ReplaceLastUpdateUntreatedPart are called from - // PseudoFdInfo::UploadBoundaryLastUntreatedArea, but not sure how to write it between - // classes, so GUARDED_BY have beed removed for now. - // - UntreatedParts untreated_list; // list of untreated parts that have been written and not yet uploaded(for streamupload) - + UntreatedParts untreated_list GUARDED_BY(fdent_lock); // list of untreated parts that have been written and not yet uploaded(for streamupload) fdinfo_map_t pseudo_fd_map GUARDED_BY(fdent_lock); // pseudo file descriptor information map FILE* pfile GUARDED_BY(fdent_lock); // file pointer(tmp file or cache file) ino_t inode GUARDED_BY(fdent_lock); // inode number for cache file @@ -87,10 +85,10 @@ class FdEntity int NoCacheLoadAndPost(PseudoFdInfo* pseudo_obj, off_t start = 0, off_t size = 0) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); // size=0 means loading to end PseudoFdInfo* CheckPseudoFdFlags(int fd, bool writable) REQUIRES(FdEntity::fdent_lock); bool IsUploading() REQUIRES(FdEntity::fdent_lock); - bool SetAllStatus(bool is_loaded) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); // [NOTE] not locking + bool SetAllStatus(bool is_loaded) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); bool SetAllStatusUnloaded() REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock) { return SetAllStatus(false); } - int NoCachePreMultipartPost(PseudoFdInfo* pseudo_obj) REQUIRES(FdEntity::fdent_lock, fdent_data_lock); - int NoCacheMultipartPost(PseudoFdInfo* pseudo_obj, int tgfd, off_t start, off_t size) REQUIRES(FdEntity::fdent_lock); + int NoCachePreMultipartPost(PseudoFdInfo* pseudo_obj) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); + int NoCacheMultipartPost(PseudoFdInfo* pseudo_obj, int tgfd, off_t start, off_t size) REQUIRES(FdEntity::fdent_lock); int NoCacheCompleteMultipartPost(PseudoFdInfo* pseudo_obj) REQUIRES(FdEntity::fdent_lock); int RowFlushHasLock(int fd, const char* tpath, bool force_sync) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); int RowFlushNoMultipart(const PseudoFdInfo* pseudo_obj, const char* tpath) REQUIRES(FdEntity::fdent_lock, FdEntity::fdent_data_lock); @@ -229,12 +227,11 @@ class FdEntity bool IsDirtyNewFile() const; void MarkDirtyMetadata(); - // [FIXME] - // For these methods, REQUIRES(FdEntity::fdent_lock) should be specified, but since it is called back - // from PseudoFdInfo::UploadBoundaryLastUntreatedArea(), don't know how to write it. - // - bool GetLastUpdateUntreatedPart(off_t& start, off_t& size) const; - bool ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size); + bool GetLastUpdateUntreatedPart(off_t& start, off_t& size) const REQUIRES(FdEntity::fdent_lock); + bool ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size) REQUIRES(FdEntity::fdent_lock); + + // Intentionally unimplemented -- for lock checking only. + std::mutex* GetMutex() RETURN_CAPABILITY(fdent_lock); }; typedef std::map> fdent_map_t; // key=path, value=unique_ptr diff --git a/src/fdcache_fdinfo.h b/src/fdcache_fdinfo.h index 72d3f54..f1d17bf 100644 --- a/src/fdcache_fdinfo.h +++ b/src/fdcache_fdinfo.h @@ -25,11 +25,11 @@ #include #include "common.h" +#include "fdcache_entity.h" #include "psemaphore.h" #include "metaheader.h" #include "types.h" -class FdEntity; class UntreatedParts; //------------------------------------------------ @@ -116,7 +116,7 @@ class PseudoFdInfo bool ParallelMultipartUploadAll(const char* path, const mp_part_list_t& to_upload_list, const mp_part_list_t& copy_list, int& result); int WaitAllThreadsExit(); - ssize_t UploadBoundaryLastUntreatedArea(const char* path, headers_t& meta, FdEntity* pfdent); + ssize_t UploadBoundaryLastUntreatedArea(const char* path, headers_t& meta, FdEntity* pfdent) REQUIRES(pfdent->GetMutex()); bool ExtractUploadPartsFromAllArea(UntreatedParts& untreated_list, mp_part_list_t& to_upload_list, mp_part_list_t& to_copy_list, mp_part_list_t& to_download_list, filepart_list_t& cancel_upload_list, bool& wait_upload_complete, off_t max_mp_size, off_t file_size, bool use_copy); }; diff --git a/src/sighandlers.cpp b/src/sighandlers.cpp index 432925f..aa3b2d1 100644 --- a/src/sighandlers.cpp +++ b/src/sighandlers.cpp @@ -22,6 +22,7 @@ #include #include +#include "psemaphore.h" #include "s3fs_logger.h" #include "sighandlers.h" #include "fdcache.h"